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

Mike Frysinger via Gdb-patches
2021-01-18T21:15:51Z

On 18 Jan 2021 13:08, Simon Marchi via Gdb-patches wrote:
> On 2021-01-18 12:52 p.m., Mike Frysinger wrote:
> > 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>
>
> Wait, what. This printf call gives you multiple lines, and does not
> tell you "you have too many arguments for the format string"? This
> is... surprising.

i guess it's just familiarity with it because it's what i expected :).
it makes the printf program pretty useful for formatting a bunch of args
like we have here.

> To produce that output, I would have expected a for loop:
>
> for f in ${files}; do
> printf '#include <%s>\n' "$f"
> done

that would work, but a single printf call is equiv & simpler :).

> > 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.
>
> Ok, I'm used to using bash arrays, I forgot they weren't standard.

hopefully one day someone will push the POSIX committe to extend the
shell language to include arrays. until then, our primitives are ...
primitive.

> > * 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.
>
> I think it would be perfectly fine to require bash or python.

in what context ? i assume it's off-limits for people downloading a
release and building e.g. gdb or gas. but is it acceptable for devs
doing dev work (which is what this thing is) ?
-mike