Re: [PATCH 1/3] commands/test: Bail out on incomplete command line options

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

On Mon, Feb 03, 2020 at 08:57:07AM +0100, Sascha Hauer wrote:
> On Tue, Jan 28, 2020 at 02:07:59PM +0100, Uwe Kleine-König wrote:
> > This makes test emit an error (and fail) on e.g.
> > 
> > 	test -f
> 
> test -f in bash returns 0 as well and also doesn't emit an error.

Ah, that's because this is implicitly

	test -n -f

That the same happens in barebox is only an accident I would say. (And
note that currently

	test ''

returns success, too, which is different from bash's test behaviour.)

> > and also on unimplemented options like
> > 
> > 	test -c /dev/null
> 
> test -H /foo/bar in bash says "bash: test: -H: unary operator expected".

That is (again) because "-H" "/dev/null" is implicitly "-n" "-H"
"/dev/null" and then there is no sane interpretation of the last
argument.

	test -H -a -f

succeeds because both "-H" and "-f" are non-zero. I'm not sure if trying
to be that clever(?) is a good idea. (Also bash's test isn't without
shortages, for example

	$ test -f -a -a -f
	bash: test: argument expected

while this is an obviously fine test if a file called "-a" exists and
"-f" is not zero-length. Also note that coreutil's test behaves differently
than bash's on for example

	test -f -a -f
	
. (bash returns success, coreutil's test wails about "extra argument
‘-f’".))

> I am not sure how relevant this is, but the behaviour you introduce is
> not consistent to bash.

While I admit I wasn't aware how strange the other test variants are, I
think trying to be as smart as bash's and coreutil's test isn't very
sensible. So I think requesting that the caller explicitly uses "-n" for
testing an argument for being non-empty is a sane idea. Not only because
it catches (probably) errors like:

	somestring=""
	if test -n $somestring; then echo some string is nonempty; fi

but also to keep the parser simpler (and the result less surprising).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox