[libgpiod,2/4] tools: tests: use $@ instead of $*

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

$@ does not break up quoted arguments which is what we want in all cases
in the bash test-suite. Use it instead of $*.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 tools/gpio-tools-test.bash | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

On Fri, May 24, 2024 at 08:03:28PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> $@ does not break up quoted arguments which is what we want in all cases
> in the bash test-suite. Use it instead of $*.
>

I believe it needs to be "$@".  Everywhere.

Where do we use quoted arguments/whitespaced parameters?
So this is purely about "good" shell?  In that case why stop here - e.g.
shellcheck picks up a load more "Double quote to prevent splitting/globbing"
and the like.

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  tools/gpio-tools-test.bash | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/gpio-tools-test.bash b/tools/gpio-tools-test.bash
> index abb2f5d..8b4f054 100755
> --- a/tools/gpio-tools-test.bash
> +++ b/tools/gpio-tools-test.bash
> @@ -27,10 +27,10 @@ GPIOSIM_APP_NAME="gpio-tools-test"
>  MIN_KERNEL_VERSION="5.17.4"
>  MIN_SHUNIT_VERSION="2.1.8"
>
> -# Run the command in $* and fail the test if the command succeeds.
> +# Run the command in $@ and fail the test if the command succeeds.
>  assert_fail() {
> -	$* || return 0
> -	fail " '$*': command did not fail as expected"
> +	$@ || return 0
> +	fail " '$@': command did not fail as expected"
>  }
>
>  # Check if the string in $2 matches against the pattern in $1.
> @@ -71,7 +71,7 @@ gpiosim_chip() {
>
>  	mkdir -p $BANKPATH
>
> -	for ARG in $*
> +	for ARG in $@
>  	do
>  		local KEY=$(echo $ARG | cut -d"=" -f1)
>  		local VAL=$(echo $ARG | cut -d"=" -f2)
> @@ -253,7 +253,7 @@ dut_regex_match() {
>  }
>
>  dut_write() {
> -	echo $* >&${COPROC[1]}
> +	echo $@ >&${COPROC[1]}
>  }
>

Does echo care?  I realise a linter, and that includes Andy, will complain
here, but is there a practical difference?

Cheers,
Kent.

>  dut_kill() {
> @@ -283,7 +283,7 @@ tearDown() {
>  }
>
>  request_release_line() {
> -	$SOURCE_DIR/gpioget -c $* >/dev/null
> +	$SOURCE_DIR/gpioget -c $@ >/dev/null
>  }
>
>  #
>
> --
> 2.43.0
>

On Sat, May 25, 2024 at 3:54 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Fri, May 24, 2024 at 08:03:28PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > $@ does not break up quoted arguments which is what we want in all cases
> > in the bash test-suite. Use it instead of $*.
> >
>
> I believe it needs to be "$@".  Everywhere.
>
> Where do we use quoted arguments/whitespaced parameters?
> So this is purely about "good" shell?  In that case why stop here - e.g.
> shellcheck picks up a load more "Double quote to prevent splitting/globbing"
> and the like.
>

You're not wrong but I have an impression that this is just a
sarcastic way of telling me this change is not needed. Could you
confirm?

> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  tools/gpio-tools-test.bash | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/gpio-tools-test.bash b/tools/gpio-tools-test.bash
> > index abb2f5d..8b4f054 100755
> > --- a/tools/gpio-tools-test.bash
> > +++ b/tools/gpio-tools-test.bash
> > @@ -27,10 +27,10 @@ GPIOSIM_APP_NAME="gpio-tools-test"
> >  MIN_KERNEL_VERSION="5.17.4"
> >  MIN_SHUNIT_VERSION="2.1.8"
> >
> > -# Run the command in $* and fail the test if the command succeeds.
> > +# Run the command in $@ and fail the test if the command succeeds.
> >  assert_fail() {
> > -     $* || return 0
> > -     fail " '$*': command did not fail as expected"
> > +     $@ || return 0
> > +     fail " '$@': command did not fail as expected"
> >  }
> >
> >  # Check if the string in $2 matches against the pattern in $1.
> > @@ -71,7 +71,7 @@ gpiosim_chip() {
> >
> >       mkdir -p $BANKPATH
> >
> > -     for ARG in $*
> > +     for ARG in $@
> >       do
> >               local KEY=$(echo $ARG | cut -d"=" -f1)
> >               local VAL=$(echo $ARG | cut -d"=" -f2)
> > @@ -253,7 +253,7 @@ dut_regex_match() {
> >  }
> >
> >  dut_write() {
> > -     echo $* >&${COPROC[1]}
> > +     echo $@ >&${COPROC[1]}
> >  }
> >
>
> Does echo care?  I realise a linter, and that includes Andy, will complain
> here, but is there a practical difference?
>

The practical difference that comes to mind is passing less arguments
to echo but I don't know if that really matters.

Bart

> Cheers,
> Kent.
>
> >  dut_kill() {
> > @@ -283,7 +283,7 @@ tearDown() {
> >  }
> >
> >  request_release_line() {
> > -     $SOURCE_DIR/gpioget -c $* >/dev/null
> > +     $SOURCE_DIR/gpioget -c $@ >/dev/null
> >  }
> >
> >  #
> >
> > --
> > 2.43.0
> >

On Mon, May 27, 2024 at 11:49:09AM +0200, Bartosz Golaszewski wrote:
> On Sat, May 25, 2024 at 3:54 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Fri, May 24, 2024 at 08:03:28PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > $@ does not break up quoted arguments which is what we want in all cases
> > > in the bash test-suite. Use it instead of $*.
> > >
> >
> > I believe it needs to be "$@".  Everywhere.
> >
> > Where do we use quoted arguments/whitespaced parameters?
> > So this is purely about "good" shell?  In that case why stop here - e.g.
> > shellcheck picks up a load more "Double quote to prevent splitting/globbing"
> > and the like.
> >
>
> You're not wrong but I have an impression that this is just a
> sarcastic way of telling me this change is not needed. Could you
> confirm?
>

Me? Sarcastic? ;-)  Well, yes and no, but mainly no.

Strictly speaking, the change is not needed, given the functions in question
are only used internally and we know whitespace is not an issue.

OTOH, I'm fine with this change, but I do think in that case we should fix
everything, to some accepted standard of "good" shell.
I believe Andy suggested the same.  I happened to suggest shellcheck as the
standard as that is what my editor happened to be using.
Happy to go with something else if you have a better alternative.

If you want to apply this series (after fixing the "$@"), I'm happy to patch
that to correct all the other things that shellcheck throws up - there are
lots.

Then if we do happen to make use of whitespace in the future we're good.

Cheers,
Kent.

On Mon, May 27, 2024 at 12:24 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Mon, May 27, 2024 at 11:49:09AM +0200, Bartosz Golaszewski wrote:
> > On Sat, May 25, 2024 at 3:54 AM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > On Fri, May 24, 2024 at 08:03:28PM +0200, Bartosz Golaszewski wrote:
> > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > >
> > > > $@ does not break up quoted arguments which is what we want in all cases
> > > > in the bash test-suite. Use it instead of $*.
> > > >
> > >
> > > I believe it needs to be "$@".  Everywhere.
> > >
> > > Where do we use quoted arguments/whitespaced parameters?
> > > So this is purely about "good" shell?  In that case why stop here - e.g.
> > > shellcheck picks up a load more "Double quote to prevent splitting/globbing"
> > > and the like.
> > >
> >
> > You're not wrong but I have an impression that this is just a
> > sarcastic way of telling me this change is not needed. Could you
> > confirm?
> >
>
> Me? Sarcastic? ;-)  Well, yes and no, but mainly no.
>
> Strictly speaking, the change is not needed, given the functions in question
> are only used internally and we know whitespace is not an issue.
>
> OTOH, I'm fine with this change, but I do think in that case we should fix
> everything, to some accepted standard of "good" shell.
> I believe Andy suggested the same.  I happened to suggest shellcheck as the
> standard as that is what my editor happened to be using.
> Happy to go with something else if you have a better alternative.
>

No, you're right, let's bring it up to shellcheck's standard.

> If you want to apply this series (after fixing the "$@"), I'm happy to patch
> that to correct all the other things that shellcheck throws up - there are
> lots.
>

Sure, knock yourself out. Resend the series with this fixed then.

> Then if we do happen to make use of whitespace in the future we're good.
>
> Cheers,
> Kent.

Thanks,
Bart