Re: [PATCH] sim: common: modernize gennltvals.sh

Mike Frysinger via Gdb-patches
2021-01-18T17:52:44Z

On 18 Jan 2021 12:19, Simon Marchi via Gdb-patches wrote:
> On 2021-01-18 12:13 p.m., Mike Frysinger wrote:
> > i had run shellcheck and the only warnings (about unquoted expansion)
> > i didn't think we worth fixing because of the limited scope of the
> > script. the variable in question isn't accepting user input, it's
> > operating on fixed inputs, and if we restrict ourselves to POSIX
> > shell (which i think we do), then our options are limited, and imo
> > the alternatives make it harder to read/understand.
>
> Even considering this, I think it's worth just quoting the variables
> and getting rid the warnings. It's trivial, and it would make any
> future (more important) warning more apparent.

to be clear, it isn't a style issue, it's a correctness issue.
adding the quotes will break the script.

here's the warning:
printf '#include <%s>\n' ${files}
^------^ SC2086: Double quote to prevent globbing and word splitting.

this is because the code passes in multiple files to process as an arg:
gentvals "" errno ... "errno.h sys/errno.h" ...

then we use it like:
files=$4
...
for f in ${files}; do
...
printf '#include <%s>\n' ${files}

unquoted, we get:
#include <errno.h>
#include <sys/errno.h>

quoted, we get:
#include <errno.h sys/errno.h>

our options are limited with POSIX shell:
* use arrays ... POSIX shell only has one builtin array: the args.
so we'd rework the func API to pass in multiple files, and we'd
operate on $@ by shifting it and iterating.
* change the shell runtime env by disabling path expansion. that
would mitigate the path expansion (which doesn't happen here as
we know the inputs are all alphanumeric/periods), but still would
have word splitting because we want that.

shell is just a bad programming language. but i think the tree
specifically constrains itself to it for portability. i don't know
what, if any, policies we have about using any other language.
-mike