Messages in this thread | | | Date | Thu, 4 Apr 2024 17:01:03 +0300 | From | Lasse Collin <> | Subject | Re: [PATCH 11/11] xz: Adjust arch-specific options for better kernel compression |
| |
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
| |