Received-SPF: pass (google.com: domain of linux-kernel+bounces-131571-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 4 Apr 2024 17:01:03 +0300
From: Lasse Collin <lasse.collin@tukaani.org>
To: <angel.lkml@16bits.net>
Cc: Andrew Morton <akpm@linux-foundation.org>, Jubin Zhong
 <zhongjubin@huawei.com>, linux-kernel@vger.kernel.org,
 vegard.nossum@oracle.com
Subject: Re: [PATCH 11/11] xz: Adjust arch-specific options for better
 kernel compression
Message-ID: <20240404170103.1bc382b3@kaneli>
In-Reply-To: <20240403225903.0773746d@kaneli>
References: <20240320183846.19475-1-lasse.collin@tukaani.org>
	<20240320183846.19475-12-lasse.collin@tukaani.org>
	<27db456edeb6f72e7e229c2333c5d8449718c26e.camel@16bits.net>
	<20240403225903.0773746d@kaneli>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On 2024-04-03 Lasse Collin wrote:
> On 2024-03-31 angel.lkml@16bits.net wrote:
> > So, in the spirit of keeping a fair amount of paranoia, and since it
> > doesn't do any harm, any such code should be failproofed to ensure
> > it can only import the expected shell variables with the right
> > format[3]:
> >
> >  eval "$($XZ --robot --version | grep
> > '^\(XZ\|LIBLZMA\)_VERSION=[0-9]*$')" || exit  
> 
> I would rather get rid of eval. I committed the following to the
> upstream repository:
> 
> XZ_VERSION=$($XZ --robot --version | sed -n 's/^XZ_VERSION=//p') ||
> exit

Both my new version and the suggested eval+grep version have error
detection issues:

  - With the eval+grep version, if there are no matches, eval gets an
    empty string as an argument in which case eval's exit status is
    zero and "exit" won't be run. Exit status from $XZ is ignored.
    XZ_VERSION won't be set or it might be inherited from the
    environment.

  - With $XZ ... | sed ..., the exit status of $XZ is ignored. sed
    will exit with 0 and thus "exit" won't be run even if $XZ fails.

Upstream I changed to this:

XZ_VERSION=$($XZ --robot --version) || exit
XZ_VERSION=$(printf '%s\n' "$XZ_VERSION" | sed -n 's/^XZ_VERSION=//p')

If output from $XZ is weird, XZ_VERSION might still become weird too.
But the way the variable is used later should at worst result in
"integer expression expected" error message.

I think the above is a good enough balance for a shell script like
this.

-- 
Lasse Collin