Re: [PATCH 11/11] xz: Adjust arch-specific options for better kernel compression

From: Lasse Collin
Date: Thu Apr 04 2024 - 10:10:59 EST


On 2024-04-03 Lasse Collin wrote:
> On 2024-03-31 angel.lkml@xxxxxxxxxx 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