Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 24 Nov 2021 09:42:41 -0700
From:      Warner Losh <imp@bsdimp.com>
To:        Mateusz Piotrowski <0mp@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>,  "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, dev-commits-src-main@freebsd.org
Subject:   Re: git: 169e06fa7852 - main - os-release: Quote variables as documented in the manual
Message-ID:  <CANCZdfq-uCfrT-0ZpqD%2BeWN47_D5cYnE6EOwbMgy6x0G=LANgg@mail.gmail.com>
In-Reply-To: <202111241528.1AOFSJdf038653@gitrepo.freebsd.org>
References:  <202111241528.1AOFSJdf038653@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000008cc16505d18b8cbb
Content-Type: text/plain; charset="UTF-8"

On Wed, Nov 24, 2021 at 8:28 AM Mateusz Piotrowski <0mp@freebsd.org> wrote:

> The branch main has been updated by 0mp (doc, ports committer):
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=169e06fa7852810f4ced2cce7a3c14ca9443bf39
>
> commit 169e06fa7852810f4ced2cce7a3c14ca9443bf39
> Author:     Mateusz Piotrowski <0mp@FreeBSD.org>
> AuthorDate: 2021-11-23 10:26:47 +0000
> Commit:     Mateusz Piotrowski <0mp@FreeBSD.org>
> CommitDate: 2021-11-24 15:17:01 +0000
>
>     os-release: Quote variables as documented in the manual
>
>     Variables must be quoted if they contain non-alphanumeric characters.
>
>     Warner noted in the review that the lack of quoting causing problems
>     here is rather an edge case. I believe that it's worth adding the
> quotes
>     here anyway because this is what the specification says and there is no
>     good reason not to follow it.
>

The only place it might be needed is for _version. Everything else can't
have troublesome characters. The manual does not say you have to
quote non-alphanumeric characters in variable assignment being my
main point. Certainly not the paths and URLs. That's just gratuitous
and not generally done in shell scripts (which I thought was a good
enough reason to not do it). I'd have preferred if that nuance had
been better reflected in the commit message since the change
itself won't break anything.

Warner


>     Reviewed by:    imp
>     Approved by:    imp (src)
>     MFC after:      7 days
> ---
>  libexec/rc/rc.d/os-release | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/libexec/rc/rc.d/os-release b/libexec/rc/rc.d/os-release
> index c328e161a70e..16f8cb178db8 100755
> --- a/libexec/rc/rc.d/os-release
> +++ b/libexec/rc/rc.d/os-release
> @@ -27,14 +27,14 @@ osrelease_start()
>         t=$(mktemp -t os-release)
>         cat > "$t" <<-__EOF__
>                 NAME=FreeBSD
> -               VERSION=$_version
> -               VERSION_ID=$_version_id
> +               VERSION="$_version"
> +               VERSION_ID="$_version_id"
>                 ID=freebsd
>                 ANSI_COLOR="0;31"
>                 PRETTY_NAME="FreeBSD $_version"
> -               CPE_NAME=cpe:/o:freebsd:freebsd:$_version_id
> -               HOME_URL=https://FreeBSD.org/
> -               BUG_REPORT_URL=https://bugs.FreeBSD.org/
> +               CPE_NAME="cpe:/o:freebsd:freebsd:$_version_id"
> +               HOME_URL="https://FreeBSD.org/"
> +               BUG_REPORT_URL="https://bugs.FreeBSD.org/"
>  __EOF__
>         install -C -o root -g wheel -m ${osrelease_perms} "$t"
> "${osrelease_file}"
>         rm -f "$t"
>

--0000000000008cc16505d18b8cbb--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfq-uCfrT-0ZpqD%2BeWN47_D5cYnE6EOwbMgy6x0G=LANgg>