spread icon indicating copy to clipboard operation
spread copied to clipboard

Fix $@ vs $* in MATCH and similar functions

Open zyga opened this issue 1 year ago • 2 comments

The variables $@ and $* differ subtly in how they are expanded.

  • $* expands to a single value with all the arguments of the program or function joined with spaces, or by the first character of the $IFS variable, to be precise.

    Usually $* is used in messages where the individual arguments and their count is irrelevant.

  • $@ is just like $*, except when in double-quoted string "$@". This form expands to multiple values, properly quoted, so that one set of arguments can be passed safely and correctly into another function.

The aforementioned macros, FATAL, ERROR, MATCH and NOMATCH all used $@ where $* was expected, and thus only really worked with a single value.

The case of MATCH and NOMATCH is more subtle, as usually a single pattern is provided. In general, $* should be used in those cases as $@ does not really do anything different in the error case (the message is the same) but the code is somewhat on the edge of correctness so I chose to adjust them as well.

zyga avatar Dec 11 '24 09:12 zyga

A few comments/questions:

  • in FATAL and ERROR $* seems to be indeed the correct way to handle multiple arguments that will be coalesced in a single argument to the real echo. Otherwise FATAL 1 2 3 will expand to echo '<FATAL: 1' 2 '3>'.
  • in MATCH and NOMATCH using $* will allow you to do things you can't do with grep, such as echo banana apple | MATCH ana app (quoting the argument is necessary in grep). In this case, isn't $@ the most appropriate variable to use, since this argument is supposed to be a regular expression and not a set of independent strings?
  • shouldn't the $stdin argument to echo in those functions also be enclosed in quotes to prevent space (or $IFS) squashing? Consider the case where echo "banana apple" | MATCH "ana app" being true could be very confusing for the user. There's also no way to reject multiple spaces in the input.
  • You provided fixes for multiple arguments in four functions, but only two are tested, and both with a single argument? It would be interesting to ensure the argument handling is correct in case of multiple arguments to FATAL and ERROR

cmatsuoka avatar Jan 25 '25 19:01 cmatsuoka

@cmatsuoka FATAL and ERROR already echo so the only reasonable thing is to use $*. FATAL 1 2 3 should expand to echo "<FATAL 1 2 3>".

zyga@x13:~$ FATAL() { echo "FATAL $*"; }; 
zyga@x13:~$ FATAL 1 2 3
FATAL 1 2 3

I think I'm convinced that MATCH can use "$@", as it retains the control of the user. I will push an update to change revert this.

I didn't think of $stdin but I think you are right.

I'll push some new tests.

Thank you for the review!

zyga avatar Jan 28 '25 10:01 zyga