Re: Proposed PATCH to allow control of the kernel ordering in grub.cfg

[Mihai Moldovan <ionic@ionic.de>]

[-- Attachment #1.1: Type: text/plain, Size: 4823 bytes --]

* On 12/27/18 3:23 PM, Jeff Norden wrote:
> Nothing is ever as simple as it seems, is it?

Never is.


> 1) Deleting the tr after the grep actually seems to break the patch.  My
> apologies for not testing this more carefully in advance.  It looks like
> the second tr masks grep's exit status, which is important b/c the
> script runs with 'set -e' and exits immediately on any "error".

Careful, that's not universally true. Some shells error out, others don't.
POSIX/SUS (at least in its current iteration) specifies that "any command" with
a few exceptions (a few reserved words, any but the last command in pipes, any
but the last command in an AND-OR list and some other exception I wasn't able to
understand even after thinking about it for half an hour) shall make the shell
fail. However, bash for instance didn't care for stuff that happens in subshells
(which backticks or the better $() notation create) before version 4, so it
seems that older versions would NOT error out if anything within a subshell
failed. Newer ones do, but there are even more caveats.

Consider
[1] (set -e; printf '%s\n' 'blargh' | grep -E 'foo'; printf '%s\n' 'bar');
printf '%s\n' "${?}"
which should print "1\n", but
[2] (set -e; ret="$(printf '%s\n' 'blargh' | grep -E 'foo'; printf '%s\n'
'bar')"; printf '%s\n' "${ret}"); printf '%s\n' "${?}"
which prints "bar\n0\n" in my bash version and then again
[3] (set -e; ret="$(set -e; printf '%s\n' 'blargh' | grep -E 'foo'; printf
'%s\n' 'bar')"; printf '%s\n' "${ret}"); printf '%s\n' "${?}"
which prints "1\n" again.

In older bash versions, any error within the subshell would have been ignored
(contrary to POSIX requirements), so in my understanding [2] and [3] would have
printed the same "bar\n0\n" string there.

The errexit shell feature can be and often is a venomous snake pit.


> Seems like changing |tr to ||true will work.

Yeah, the usual workaround for that is either "cmdthatmightfail || true" or
"cmdthatmightfail || :"


> 2) After reading Mihai Moldovan's comments above, I think it is better
> to just use grep rather than egrep.  You could always add "-E" at the
> beginning of the GRUB_PREFERRED_KERNEL string if you need to.  With GNU
> grep, egrep and grep have the same functionality, and only differ in
> when you need to backslash-escape some of the special characters.

No, don't even start with that. Either use
egrep "${GRUB_PREFERRED_KERNEL}"
or
grep -E "${GRUB_PREFERRED_KERNEL}"

Why?

ionic@apgunner ~/ $  input="-E 4.20"; grep "${input}" '/dev/null'
grep: invalid option -- ' '
Usage: grep [OPTION]... PATTERN [FILE]...
Try 'grep --help' for more information.

In such a scenario, because the input string is quoted, it will be passed as one
single argument to the grep utility, which then tries to parse the full string
as one huge option. Take into consideration that short options can be usually
concatenated together, so grep's option parsing tries to adhere to this as soon
as it saw the short option starting sequence "-". Funnily enough, a short option
such as "- " might just be valid. It just turns out that grep doesn't support it.

A remedy to this could be to prefix any user-provided input with the
options-terminating pseudo-option "--" (often called delimiter).

input="-E 4.20"; grep -- "${input}" /dev/null

would work "correctly" (if you really *intended* to search for a string such as
"-E 4.20" or "-E 4,20" or "-E 4a20" or ...), but also not treat anything that
might resemble an option in the user string as a real option, which usually is
the right thing to do. Input sanitization or lack thereof is a huge problem. You
generally do not want users to pass random options to commands, for the reasons
you have outlined yourself in point 3.

The problem is just that command line parsing is difficult and while "--" is an
accepted delimiter for getopt() and friends, there are utilities that don't even
use getopt(), but instead roll their custom solution that happens to not support
that. Another common exception to this is bash's "echo" builtin which explicitly
does not handle "--" as a delimiter. Also, people are lazy and don't like to
clutter every command invocation with a delimiter after specifying wanted
options and even when they actually try to, it's oh-so-easy to forget.

To my mind, grep -E -- "${GRUB_PREFERRED_KERNEL}" sounds like the best solution.
You obviously already took into account that dots in regular expressions match
arbitrary characters and outlined that, including consequences, in the
documentation; good work on that!


Originally, I was just trying to point out that I'd prefer to use "grep -E"
nowadays compared to "egrep" - seems like we went off the rail pretty quickly. :)



Mihai


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 898 bytes --]