* [PATCH 1/3] commands/test: Bail out on incomplete command line options
2020-01-28 13:07 [PATCH 0/3] commands/test: some improvements Uwe Kleine-König
@ 2020-01-28 13:07 ` Uwe Kleine-König
2020-02-03 7:57 ` Sascha Hauer
2020-01-28 13:08 ` [PATCH 2/3] commands/test: Improve option parsing to handle "]" less special Uwe Kleine-König
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2020-01-28 13:07 UTC (permalink / raw)
To: barebox
This makes test emit an error (and fail) on e.g.
test -f
and also on unimplemented options like
test -c /dev/null
.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
commands/test.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/commands/test.c b/commands/test.c
index c4f493860f14..ab108fc845d5 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -208,6 +208,11 @@ static int do_test(int argc, char *argv[])
}
}
+ if (left < adv) {
+ printf("test: failed to parse arguments\n");
+ return 1;
+ }
+
if (last_cmp == 0)
expr = last_expr || expr;
else if (last_cmp == 1)
--
2.24.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] commands/test: Bail out on incomplete command line options
2020-01-28 13:07 ` [PATCH 1/3] commands/test: Bail out on incomplete command line options Uwe Kleine-König
@ 2020-02-03 7:57 ` Sascha Hauer
2020-02-03 8:50 ` Uwe Kleine-König
0 siblings, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2020-02-03 7:57 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: barebox
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.
>
> and also on unimplemented options like
>
> test -c /dev/null
test -H /foo/bar in bash says "bash: test: -H: unary operator expected".
I am not sure how relevant this is, but the behaviour you introduce is
not consistent to bash.
Sascha
>
> .
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> commands/test.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/commands/test.c b/commands/test.c
> index c4f493860f14..ab108fc845d5 100644
> --- a/commands/test.c
> +++ b/commands/test.c
> @@ -208,6 +208,11 @@ static int do_test(int argc, char *argv[])
> }
> }
>
> + if (left < adv) {
> + printf("test: failed to parse arguments\n");
> + return 1;
> + }
> +
> if (last_cmp == 0)
> expr = last_expr || expr;
> else if (last_cmp == 1)
> --
> 2.24.0
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] commands/test: Bail out on incomplete command line options
2020-02-03 7:57 ` Sascha Hauer
@ 2020-02-03 8:50 ` Uwe Kleine-König
0 siblings, 0 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2020-02-03 8:50 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] commands/test: Improve option parsing to handle "]" less special
2020-01-28 13:07 [PATCH 0/3] commands/test: some improvements Uwe Kleine-König
2020-01-28 13:07 ` [PATCH 1/3] commands/test: Bail out on incomplete command line options Uwe Kleine-König
@ 2020-01-28 13:08 ` Uwe Kleine-König
2020-01-28 13:08 ` [PATCH 3/3] commands/test: Implement -b and -c to test for character and block devices Uwe Kleine-König
2020-02-04 7:40 ` [PATCH 0/3] commands/test: some improvements Sascha Hauer
3 siblings, 0 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2020-01-28 13:08 UTC (permalink / raw)
To: barebox
Testing for *ap[1] != ']' is bogus during option processing. Instead
test if there are options left to be processed.
This fixes the return value for e.g.
test -z ']lala'
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
commands/test.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/commands/test.c b/commands/test.c
index ab108fc845d5..9070e4907496 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -129,8 +129,11 @@ static int do_test(int argc, char *argv[])
case OPT_ZERO:
case OPT_NONZERO:
adv = 2;
+ if (left < 2)
+ break;
zero = 1;
- if (ap[1] && *ap[1] != ']' && strlen(ap[1]))
+
+ if (strlen(ap[1]))
zero = 0;
expr = (opt == OPT_ZERO) ? zero : !zero;
@@ -141,7 +144,9 @@ static int do_test(int argc, char *argv[])
case OPT_EXISTS:
case OPT_SYMBOLIC_LINK:
adv = 2;
- if (ap[1] && *ap[1] != ']' && strlen(ap[1])) {
+ if (left < 2)
+ break;
+ if (strlen(ap[1])) {
expr = (opt == OPT_SYMBOLIC_LINK ? lstat : stat)(ap[1], &statbuf);
if (expr < 0) {
expr = 0;
@@ -170,10 +175,8 @@ static int do_test(int argc, char *argv[])
/* three argument options */
default:
adv = 3;
- if (left < 3) {
- expr = 1;
+ if (left < 3)
break;
- }
a = simple_strtol(ap[0], NULL, 0);
b = simple_strtol(ap[2], NULL, 0);
--
2.24.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 3/3] commands/test: Implement -b and -c to test for character and block devices
2020-01-28 13:07 [PATCH 0/3] commands/test: some improvements Uwe Kleine-König
2020-01-28 13:07 ` [PATCH 1/3] commands/test: Bail out on incomplete command line options Uwe Kleine-König
2020-01-28 13:08 ` [PATCH 2/3] commands/test: Improve option parsing to handle "]" less special Uwe Kleine-König
@ 2020-01-28 13:08 ` Uwe Kleine-König
2020-02-04 7:40 ` [PATCH 0/3] commands/test: some improvements Sascha Hauer
3 siblings, 0 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2020-01-28 13:08 UTC (permalink / raw)
To: barebox
These match the same options on coreutil's test(1).
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
commands/test.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/commands/test.c b/commands/test.c
index 9070e4907496..86636de1c283 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -40,6 +40,8 @@ typedef enum {
OPT_DIRECTORY,
OPT_FILE,
OPT_EXISTS,
+ OPT_BLOCK,
+ OPT_CHAR,
OPT_SYMBOLIC_LINK,
OPT_MAX,
} test_opts;
@@ -60,6 +62,8 @@ static char *test_options[] = {
[OPT_FILE] = "-f",
[OPT_DIRECTORY] = "-d",
[OPT_EXISTS] = "-e",
+ [OPT_BLOCK] = "-b",
+ [OPT_CHAR] = "-c",
[OPT_SYMBOLIC_LINK] = "-L",
};
@@ -142,6 +146,8 @@ static int do_test(int argc, char *argv[])
case OPT_FILE:
case OPT_DIRECTORY:
case OPT_EXISTS:
+ case OPT_BLOCK:
+ case OPT_CHAR:
case OPT_SYMBOLIC_LINK:
adv = 2;
if (left < 2)
@@ -169,6 +175,14 @@ static int do_test(int argc, char *argv[])
expr = 1;
break;
}
+ if (opt == OPT_BLOCK && S_ISBLK(statbuf.st_mode)) {
+ expr = 1;
+ break;
+ }
+ if (opt == OPT_CHAR && S_ISCHR(statbuf.st_mode)) {
+ expr = 1;
+ break;
+ }
}
break;
--
2.24.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/3] commands/test: some improvements
2020-01-28 13:07 [PATCH 0/3] commands/test: some improvements Uwe Kleine-König
` (2 preceding siblings ...)
2020-01-28 13:08 ` [PATCH 3/3] commands/test: Implement -b and -c to test for character and block devices Uwe Kleine-König
@ 2020-02-04 7:40 ` Sascha Hauer
3 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2020-02-04 7:40 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: barebox
On Tue, Jan 28, 2020 at 02:07:58PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> my main intention was to implement -b and -c, the other two patches just
> fix some stuff I noticed while doing so.
>
> One thing I noticed is that the check for
>
> strlen(ap[1])
>
> in the handling of -e and others is wrong.
>
> This makes
>
> test -e ''
>
> return success, but
>
> test -z something -o -e ''
>
> fails. I don't know for sure how to fix this as the empty string might
> mean "." sometimes?
>
> Best regards
> Uwe
>
> Uwe Kleine-König (3):
> commands/test: Bail out on incomplete command line options
> commands/test: Improve option parsing to handle "]" less special
> commands/test: Implement -b and -c to test for character and block
> devices
>
> commands/test.c | 32 +++++++++++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 5 deletions(-)
Applied, thanks
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 7+ messages in thread