Re: [PATCH libgpiod v2 2/4] tools: tests: use "$@" instead of $*

[Kent Gibson <warthog618@gmail.com>]

On Wed, May 29, 2024 at 04:08:49PM +0300, Andy Shevchenko wrote:
> On Tue, May 28, 2024 at 07:39:10AM +0800, Kent Gibson wrote:
> > On Mon, May 27, 2024 at 07:17:37PM +0300, Andy Shevchenko wrote:
> > > On Mon, May 27, 2024 at 08:44:20PM +0800, Kent Gibson wrote:
> > > > On Mon, May 27, 2024 at 02:02:34PM +0200, Bartosz Golaszewski wrote:
>
> ...
>
> > > > >  assert_fail() {
> > > > > -	$* || return 0
> > > > > -	fail " '$*': command did not fail as expected"
> > > > > +	"$@" || return 0
> > > > > +	fail " '$@': command did not fail as expected"
> > > > >  }
> > > >
> > > > Ironically, shellcheck doesn't like the '$@' in the fail string[1], so you
> > > > should use $* there.
> > >
> > > But why does it do like this?
> >
> > Read the link[1].
>
> Okay, this is only for some debug / error messages. Still if one wants to have
> clear understanding on what has been passed to some function, $* is not a
> correct option. Also note the single quotes, shouldn't that protect from the
> arguments loss?
>

That's right - I was only referring to this particular case where a
string is being constructed.  Wasn't that clear?

The single quotes are within double quotes, so aren't they just part of
the text in this context?

> > Because $@ is an array being used to build a string, and that may not
> > work the way you expect.
>
> I think it's the opposite, $* works in a way I do not expect :-)
>

When passing arguments, sure.  Not when constructing strings.

> > In this case $* is clearer as that has already
> > been concatenated.
>
> ...loosing information about which word refers to which argument, yes.
>

It is building a string, so arguments are irrelevant.


> > [1] https://www.shellcheck.net/wiki/SC2145
>
> TL;DR: I consider this is still a bug in shellcheck. But if you rely on the
> tool as on the ruleset carved in stone, I will not die. Just a remark to
> myself "even honourable tools may also be broken".
>

If you think it is a bug then raise it with shellcheck.
I think you are conflating cases, and I agree with shellcheck on this one.

Cheers,
Kent.