* [PATCH 0/3] add [ as alias for test, fix 0/1 argument handling
@ 2026-03-11 12:09 Rasmus Villemoes
2026-03-11 12:09 ` [PATCH 1/3] cmd: test: allow using [ as alias for test Rasmus Villemoes
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Rasmus Villemoes @ 2026-03-11 12:09 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Rasmus Villemoes
Make 'test' behave a little more like its cousins in other shells, by
allowing the [ ... ] spelling, and while here, fix up the handling of
a single, non-empty argument to comply with POSIX.
Rasmus Villemoes (3):
cmd: test: allow using [ as alias for test
test: add tests for left-bracket alias for 'test' command
cmd: test: fix handling of single-argument form of test
cmd/test.c | 30 +++++++++++++++++++++++--
test/hush/if.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 89 insertions(+), 2 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] cmd: test: allow using [ as alias for test 2026-03-11 12:09 [PATCH 0/3] add [ as alias for test, fix 0/1 argument handling Rasmus Villemoes @ 2026-03-11 12:09 ` Rasmus Villemoes 2026-03-11 13:08 ` Quentin Schulz 2026-03-11 12:09 ` [PATCH 2/3] test: add tests for left-bracket alias for 'test' command Rasmus Villemoes 2026-03-11 12:09 ` [PATCH 3/3] cmd: test: fix handling of single-argument form of test Rasmus Villemoes 2 siblings, 1 reply; 12+ messages in thread From: Rasmus Villemoes @ 2026-03-11 12:09 UTC (permalink / raw) To: u-boot; +Cc: Tom Rini, Rasmus Villemoes I often find myself writing something like if [ "$somevar" = 123 ] ; then ... only to realize that that syntax doesn't work in U-Boot shell, and must be spelled if test "$somevar" = 123 ; then It only takes a few lines of code to support this POSIX-standardized alias for test, and helps developers focus on their actual problems instead of dealing with such unexpected quirks of the shell. Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> --- cmd/test.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/cmd/test.c b/cmd/test.c index a9ac07e6143..f0645ef98f1 100644 --- a/cmd/test.c +++ b/cmd/test.c @@ -63,6 +63,14 @@ static int do_test(struct cmd_tbl *cmdtp, int flag, int argc, char * const *ap; int i, op, left, adv, expr, last_expr, last_unop, last_binop; + if (!strcmp(argv[0], "[")) { + if (strcmp(argv[argc - 1], "]")) { + printf("[: missing terminating ]\n"); + return 1; + } + argc--; + } + /* args? */ if (argc < 3) return 1; @@ -212,6 +220,17 @@ U_BOOT_CMD( "[args..]" ); +/* + * This does not use the U_BOOT_CMD macro as [ can't be used in symbol names + */ +ll_entry_declare(struct cmd_tbl, lbracket, cmd) = { + "[", CONFIG_SYS_MAXARGS, cmd_always_repeatable, do_test, + "alias for 'test'", +#ifdef CONFIG_SYS_LONGHELP + " <test expression> ]" +#endif /* CONFIG_SYS_LONGHELP */ +}; + static int do_false(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] cmd: test: allow using [ as alias for test 2026-03-11 12:09 ` [PATCH 1/3] cmd: test: allow using [ as alias for test Rasmus Villemoes @ 2026-03-11 13:08 ` Quentin Schulz 2026-03-11 13:25 ` Rasmus Villemoes 0 siblings, 1 reply; 12+ messages in thread From: Quentin Schulz @ 2026-03-11 13:08 UTC (permalink / raw) To: Rasmus Villemoes, u-boot; +Cc: Tom Rini Hi Rasmus, On 3/11/26 1:09 PM, Rasmus Villemoes wrote: > I often find myself writing something like > > if [ "$somevar" = 123 ] ; then ... > > only to realize that that syntax doesn't work in U-Boot shell, and > must be spelled > > if test "$somevar" = 123 ; then > > It only takes a few lines of code to support this POSIX-standardized > alias for test, and helps developers focus on their actual problems > instead of dealing with such unexpected quirks of the shell. > > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > --- > cmd/test.c | 19 +++++++++++++++++++ Please don't forget to update the docs. It still states "Unlike in ordinary shells, it cannot be spelled [.", c.f. https://docs.u-boot.org/en/latest/usage/cmd/test.html#description. The synopsis should also have a few examples using the new syntax. Another question is if you have thought about/tested multi-expressions? e.g. [ aaa = aaa -a bbb = bbb ] and possibly also [ ! aaa = aaa ] which I'm assuming should be equivalent to [ aaa != aaa ] (of course this test doesn't make much sense, but there seems to be support for the -e operator for checking file existence). I think adding tests for those would be nice too? Cheers, Quentin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] cmd: test: allow using [ as alias for test 2026-03-11 13:08 ` Quentin Schulz @ 2026-03-11 13:25 ` Rasmus Villemoes 0 siblings, 0 replies; 12+ messages in thread From: Rasmus Villemoes @ 2026-03-11 13:25 UTC (permalink / raw) To: Quentin Schulz; +Cc: u-boot, Tom Rini On Wed, Mar 11 2026, Quentin Schulz <quentin.schulz@cherry.de> wrote: > Hi Rasmus, > > On 3/11/26 1:09 PM, Rasmus Villemoes wrote: >> I often find myself writing something like >> if [ "$somevar" = 123 ] ; then ... >> only to realize that that syntax doesn't work in U-Boot shell, and >> must be spelled >> if test "$somevar" = 123 ; then >> It only takes a few lines of code to support this POSIX-standardized >> alias for test, and helps developers focus on their actual problems >> instead of dealing with such unexpected quirks of the shell. >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> >> --- >> cmd/test.c | 19 +++++++++++++++++++ > > Please don't forget to update the docs. It still states "Unlike in > ordinary shells, it cannot be spelled [.", > c.f. https://docs.u-boot.org/en/latest/usage/cmd/test.html#description. The > synopsis should also have a few examples using the new syntax. Ah, yes, thanks. Rather amazing that I could complete forget having written that only 10 months ago... Will update. > Another question is if you have thought about/tested multi-expressions? > > e.g. > > [ aaa = aaa -a bbb = bbb ] > > and possibly also > > [ ! aaa = aaa ] > Yes, I did think about those, and I can certainly copy-paste a few more test cases and spell them using [], but as the implementation is so simple (check last argv, decrement argc) I couldn't really imagine those being any different than the simple cases. > I think adding tests for those would be nice too? Sure, will do. Rasmus ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] test: add tests for left-bracket alias for 'test' command 2026-03-11 12:09 [PATCH 0/3] add [ as alias for test, fix 0/1 argument handling Rasmus Villemoes 2026-03-11 12:09 ` [PATCH 1/3] cmd: test: allow using [ as alias for test Rasmus Villemoes @ 2026-03-11 12:09 ` Rasmus Villemoes 2026-03-16 2:43 ` Simon Glass 2026-03-11 12:09 ` [PATCH 3/3] cmd: test: fix handling of single-argument form of test Rasmus Villemoes 2 siblings, 1 reply; 12+ messages in thread From: Rasmus Villemoes @ 2026-03-11 12:09 UTC (permalink / raw) To: u-boot; +Cc: Tom Rini, Rasmus Villemoes Duplicate a few of the existing test cases, using the [ spelling, and also ensure that the presence of a matching ] as a separate and last argument is enforced. Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> --- test/hush/if.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/hush/if.c b/test/hush/if.c index ea615b246a9..148e9a53e90 100644 --- a/test/hush/if.c +++ b/test/hush/if.c @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts) return 0; } HUSH_TEST(hush_test_if_z_operator, 0); + +static int hush_test_lbracket_alias(struct unit_test_state *uts) +{ + char if_formatted[128]; + const char *missing_rbracket_error = "[: missing terminating ]"; + + sprintf(if_formatted, if_format, "[ aaa = aaa ]"); + ut_assertok(run_command(if_formatted, 0)); + + sprintf(if_formatted, if_format, "[ aaa = bbb ]"); + ut_asserteq(1, run_command(if_formatted, 0)); + + sprintf(if_formatted, if_format, "[ aaa = aaa"); + ut_asserteq(1, run_command(if_formatted, 0)); + ut_assert_nextline(missing_rbracket_error); + + sprintf(if_formatted, if_format, "[ aaa = bbb"); + ut_asserteq(1, run_command(if_formatted, 0)); + ut_assert_nextline(missing_rbracket_error); + + sprintf(if_formatted, if_format, "[ aaa = aaa]"); + ut_asserteq(1, run_command(if_formatted, 0)); + ut_assert_nextline(missing_rbracket_error); + + sprintf(if_formatted, if_format, "[ aaa = bbb]"); + ut_asserteq(1, run_command(if_formatted, 0)); + ut_assert_nextline(missing_rbracket_error); + + return 0; +} +HUSH_TEST(hush_test_lbracket_alias, UTF_CONSOLE); -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command 2026-03-11 12:09 ` [PATCH 2/3] test: add tests for left-bracket alias for 'test' command Rasmus Villemoes @ 2026-03-16 2:43 ` Simon Glass 2026-03-16 8:16 ` Rasmus Villemoes 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2026-03-16 2:43 UTC (permalink / raw) To: Rasmus Villemoes; +Cc: u-boot, Tom Rini Hi Rasmus, On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote: > > Duplicate a few of the existing test cases, using the [ spelling, and > also ensure that the presence of a matching ] as a separate and last > argument is enforced. > > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > --- > test/hush/if.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/test/hush/if.c b/test/hush/if.c > index ea615b246a9..148e9a53e90 100644 > --- a/test/hush/if.c > +++ b/test/hush/if.c > @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts) > return 0; > } > HUSH_TEST(hush_test_if_z_operator, 0); > + > +static int hush_test_lbracket_alias(struct unit_test_state *uts) > +{ > + char if_formatted[128]; > + const char *missing_rbracket_error = "[: missing terminating ]"; > + > + sprintf(if_formatted, if_format, "[ aaa = aaa ]"); > + ut_assertok(run_command(if_formatted, 0)); How about using run_commandf() so you can do this in one line? Looks good apart from that. > + > + sprintf(if_formatted, if_format, "[ aaa = bbb ]"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + > + sprintf(if_formatted, if_format, "[ aaa = aaa"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + ut_assert_nextline(missing_rbracket_error); > + > + sprintf(if_formatted, if_format, "[ aaa = bbb"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + ut_assert_nextline(missing_rbracket_error); > + > + sprintf(if_formatted, if_format, "[ aaa = aaa]"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + ut_assert_nextline(missing_rbracket_error); > + > + sprintf(if_formatted, if_format, "[ aaa = bbb]"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + ut_assert_nextline(missing_rbracket_error); > + > + return 0; > +} > +HUSH_TEST(hush_test_lbracket_alias, UTF_CONSOLE); > -- > 2.53.0 > Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command 2026-03-16 2:43 ` Simon Glass @ 2026-03-16 8:16 ` Rasmus Villemoes 2026-03-17 12:28 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Rasmus Villemoes @ 2026-03-16 8:16 UTC (permalink / raw) To: Simon Glass; +Cc: u-boot, Tom Rini On Sun, Mar 15 2026, Simon Glass <sjg@chromium.org> wrote: > Hi Rasmus, > > On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote: >> >> Duplicate a few of the existing test cases, using the [ spelling, and >> also ensure that the presence of a matching ] as a separate and last >> argument is enforced. >> >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> >> --- >> test/hush/if.c | 31 +++++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/test/hush/if.c b/test/hush/if.c >> index ea615b246a9..148e9a53e90 100644 >> --- a/test/hush/if.c >> +++ b/test/hush/if.c >> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts) >> return 0; >> } >> HUSH_TEST(hush_test_if_z_operator, 0); >> + >> +static int hush_test_lbracket_alias(struct unit_test_state *uts) >> +{ >> + char if_formatted[128]; >> + const char *missing_rbracket_error = "[: missing terminating ]"; >> + >> + sprintf(if_formatted, if_format, "[ aaa = aaa ]"); >> + ut_assertok(run_command(if_formatted, 0)); > > How about using run_commandf() so you can do this in one line? Looks > good apart from that. > I did cringe a little when I saw that repeated sprintf()/run_command() pattern all over that file, but I preferred to stay consistent with the existing style. If anything, one should do a whole-sale conversion of that file, but then I'd not use run_commandf() directly, as one would still pass in that if_format argument every time, so rather create a local macro that wraps run_commandf() and provides that if_format as a literal string, which would also enable format checking. Rasmus ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command 2026-03-16 8:16 ` Rasmus Villemoes @ 2026-03-17 12:28 ` Simon Glass 2026-03-17 15:11 ` Rasmus Villemoes 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2026-03-17 12:28 UTC (permalink / raw) To: Rasmus Villemoes; +Cc: u-boot, Tom Rini Hi Rasmus, On Mon, 16 Mar 2026 at 02:17, Rasmus Villemoes <ravi@prevas.dk> wrote: > > On Sun, Mar 15 2026, Simon Glass <sjg@chromium.org> wrote: > > > Hi Rasmus, > > > > On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote: > >> > >> Duplicate a few of the existing test cases, using the [ spelling, and > >> also ensure that the presence of a matching ] as a separate and last > >> argument is enforced. > >> > >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > >> --- > >> test/hush/if.c | 31 +++++++++++++++++++++++++++++++ > >> 1 file changed, 31 insertions(+) > >> > >> diff --git a/test/hush/if.c b/test/hush/if.c > >> index ea615b246a9..148e9a53e90 100644 > >> --- a/test/hush/if.c > >> +++ b/test/hush/if.c > >> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts) > >> return 0; > >> } > >> HUSH_TEST(hush_test_if_z_operator, 0); > >> + > >> +static int hush_test_lbracket_alias(struct unit_test_state *uts) > >> +{ > >> + char if_formatted[128]; > >> + const char *missing_rbracket_error = "[: missing terminating ]"; > >> + > >> + sprintf(if_formatted, if_format, "[ aaa = aaa ]"); > >> + ut_assertok(run_command(if_formatted, 0)); > > > > How about using run_commandf() so you can do this in one line? Looks > > good apart from that. > > > > I did cringe a little when I saw that repeated sprintf()/run_command() > pattern all over that file, but I preferred to stay consistent with the > existing style. > > If anything, one should do a whole-sale conversion of that file, but > then I'd not use run_commandf() directly, as one would still pass in > that if_format argument every time, so rather create a local macro that > wraps run_commandf() and provides that if_format as a literal string, > which would also enable format checking. Well you could have a helper function which takes the string argument and does the assert. The duplication makes it a bit harder to maintain. sprintf(if_formatted, if_format, "test aaa != aaa"); ut_asserteq(1, run_command(if_formatted, 0)); could be something like: ut_asserteq(1, check_operation("test aaa != aaa")); If you had the energy to adjust the rest of the file, that seems good too. Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command 2026-03-17 12:28 ` Simon Glass @ 2026-03-17 15:11 ` Rasmus Villemoes 2026-03-18 1:03 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Rasmus Villemoes @ 2026-03-17 15:11 UTC (permalink / raw) To: Simon Glass; +Cc: u-boot, Tom Rini On Tue, Mar 17 2026, Simon Glass <sjg@chromium.org> wrote: > Hi Rasmus, > > On Mon, 16 Mar 2026 at 02:17, Rasmus Villemoes <ravi@prevas.dk> wrote: >> >> On Sun, Mar 15 2026, Simon Glass <sjg@chromium.org> wrote: >> >> > Hi Rasmus, >> > >> > On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote: >> >> >> >> Duplicate a few of the existing test cases, using the [ spelling, and >> >> also ensure that the presence of a matching ] as a separate and last >> >> argument is enforced. >> >> >> >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> >> >> --- >> >> test/hush/if.c | 31 +++++++++++++++++++++++++++++++ >> >> 1 file changed, 31 insertions(+) >> >> >> >> diff --git a/test/hush/if.c b/test/hush/if.c >> >> index ea615b246a9..148e9a53e90 100644 >> >> --- a/test/hush/if.c >> >> +++ b/test/hush/if.c >> >> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts) >> >> return 0; >> >> } >> >> HUSH_TEST(hush_test_if_z_operator, 0); >> >> + >> >> +static int hush_test_lbracket_alias(struct unit_test_state *uts) >> >> +{ >> >> + char if_formatted[128]; >> >> + const char *missing_rbracket_error = "[: missing terminating ]"; >> >> + >> >> + sprintf(if_formatted, if_format, "[ aaa = aaa ]"); >> >> + ut_assertok(run_command(if_formatted, 0)); >> > >> > How about using run_commandf() so you can do this in one line? Looks >> > good apart from that. >> > >> >> I did cringe a little when I saw that repeated sprintf()/run_command() >> pattern all over that file, but I preferred to stay consistent with the >> existing style. >> >> If anything, one should do a whole-sale conversion of that file, but >> then I'd not use run_commandf() directly, as one would still pass in >> that if_format argument every time, so rather create a local macro that >> wraps run_commandf() and provides that if_format as a literal string, >> which would also enable format checking. > > Well you could have a helper function which takes the string argument > and does the assert. The duplication makes it a bit harder to > maintain. > > sprintf(if_formatted, if_format, "test aaa != aaa"); > ut_asserteq(1, run_command(if_formatted, 0)); > > could be something like: > > ut_asserteq(1, check_operation("test aaa != aaa")); Yes, I'd definitely make sure that each test would only occupy one source line instead of two. What I meant was that I didn't want to convert sprintf(if_formatted, if_format, "test aaa = aaa"); ut_assertok(run_command(if_formatted, 0)); mechanically into ut_assertok(run_commandf(if_format, "test aaa = aaa", 0)); and having to repeat that if_format thing on each and every line. Also, I think I would prefer to change the assertok()s into asserteq(0, ... ) as I think the mix of assertok() and asserteq(1, ...) is not very readable. > If you had the energy to adjust the rest of the file, that seems good too. I will fix that in a followup if the current patches get merged (they're at v2 by the way), I do not have the energy to start over and doing the mechanical changes first and then redo my patches on top of that, so please don't request that I do that. Rasmus ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command 2026-03-17 15:11 ` Rasmus Villemoes @ 2026-03-18 1:03 ` Simon Glass 0 siblings, 0 replies; 12+ messages in thread From: Simon Glass @ 2026-03-18 1:03 UTC (permalink / raw) To: Rasmus Villemoes; +Cc: u-boot, Tom Rini Hi Rasmus, On Tue, 17 Mar 2026 at 09:12, Rasmus Villemoes <ravi@prevas.dk> wrote: > > On Tue, Mar 17 2026, Simon Glass <sjg@chromium.org> wrote: > > > Hi Rasmus, > > > > On Mon, 16 Mar 2026 at 02:17, Rasmus Villemoes <ravi@prevas.dk> wrote: > >> > >> On Sun, Mar 15 2026, Simon Glass <sjg@chromium.org> wrote: > >> > >> > Hi Rasmus, > >> > > >> > On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote: > >> >> > >> >> Duplicate a few of the existing test cases, using the [ spelling, and > >> >> also ensure that the presence of a matching ] as a separate and last > >> >> argument is enforced. > >> >> > >> >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > >> >> --- > >> >> test/hush/if.c | 31 +++++++++++++++++++++++++++++++ > >> >> 1 file changed, 31 insertions(+) > >> >> > >> >> diff --git a/test/hush/if.c b/test/hush/if.c > >> >> index ea615b246a9..148e9a53e90 100644 > >> >> --- a/test/hush/if.c > >> >> +++ b/test/hush/if.c > >> >> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts) > >> >> return 0; > >> >> } > >> >> HUSH_TEST(hush_test_if_z_operator, 0); > >> >> + > >> >> +static int hush_test_lbracket_alias(struct unit_test_state *uts) > >> >> +{ > >> >> + char if_formatted[128]; > >> >> + const char *missing_rbracket_error = "[: missing terminating ]"; > >> >> + > >> >> + sprintf(if_formatted, if_format, "[ aaa = aaa ]"); > >> >> + ut_assertok(run_command(if_formatted, 0)); > >> > > >> > How about using run_commandf() so you can do this in one line? Looks > >> > good apart from that. > >> > > >> > >> I did cringe a little when I saw that repeated sprintf()/run_command() > >> pattern all over that file, but I preferred to stay consistent with the > >> existing style. > >> > >> If anything, one should do a whole-sale conversion of that file, but > >> then I'd not use run_commandf() directly, as one would still pass in > >> that if_format argument every time, so rather create a local macro that > >> wraps run_commandf() and provides that if_format as a literal string, > >> which would also enable format checking. > > > > Well you could have a helper function which takes the string argument > > and does the assert. The duplication makes it a bit harder to > > maintain. > > > > sprintf(if_formatted, if_format, "test aaa != aaa"); > > ut_asserteq(1, run_command(if_formatted, 0)); > > > > could be something like: > > > > ut_asserteq(1, check_operation("test aaa != aaa")); > > Yes, I'd definitely make sure that each test would only occupy one > source line instead of two. What I meant was that I didn't want to > convert > > sprintf(if_formatted, if_format, "test aaa = aaa"); > ut_assertok(run_command(if_formatted, 0)); > > mechanically into > > ut_assertok(run_commandf(if_format, "test aaa = aaa", 0)); > > and having to repeat that if_format thing on each and every line. > > Also, I think I would prefer to change the assertok()s into asserteq(0, > ... ) as I think the mix of assertok() and asserteq(1, ...) is not very > readable. > > > If you had the energy to adjust the rest of the file, that seems good too. > > I will fix that in a followup if the current patches get merged (they're > at v2 by the way), I do not have the energy to start over and doing the > mechanical changes first and then redo my patches on top of that, so > please don't request that I do that. That's fine with me, please do. Yes I think I missed v1. Reviewed-by: Simon Glass <sjg@chromium.org> Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] cmd: test: fix handling of single-argument form of test 2026-03-11 12:09 [PATCH 0/3] add [ as alias for test, fix 0/1 argument handling Rasmus Villemoes 2026-03-11 12:09 ` [PATCH 1/3] cmd: test: allow using [ as alias for test Rasmus Villemoes 2026-03-11 12:09 ` [PATCH 2/3] test: add tests for left-bracket alias for 'test' command Rasmus Villemoes @ 2026-03-11 12:09 ` Rasmus Villemoes 2026-03-16 2:43 ` Simon Glass 2 siblings, 1 reply; 12+ messages in thread From: Rasmus Villemoes @ 2026-03-11 12:09 UTC (permalink / raw) To: u-boot; +Cc: Tom Rini, Rasmus Villemoes POSIX states that 0 arguments: Exit false (1). 1 argument: Exit true (0) if $1 is not null; otherwise, exit false. and at least bash and busybox sh behave that way. The current 'argc < 3' does the right thing for a non-existing or empty argv[1], but not for a non-empty argv[1]. Fix that and add corresponding test cases. Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> --- cmd/test.c | 11 +++++++++-- test/hush/if.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/cmd/test.c b/cmd/test.c index f0645ef98f1..0d0f090386c 100644 --- a/cmd/test.c +++ b/cmd/test.c @@ -71,10 +71,17 @@ static int do_test(struct cmd_tbl *cmdtp, int flag, int argc, argc--; } - /* args? */ - if (argc < 3) + /* + * Per POSIX, 'test' with 0 arguments should return 1, while + * 'test <arg>' should be equivalent to 'test -n <arg>', + * i.e. true if and only if <arg> is not empty. + */ + if (argc < 2) return 1; + if (argc == 2) + return !strcmp(argv[1], ""); + #ifdef DEBUG { debug("test(%d):", argc); diff --git a/test/hush/if.c b/test/hush/if.c index 148e9a53e90..e82a95aaf27 100644 --- a/test/hush/if.c +++ b/test/hush/if.c @@ -32,6 +32,15 @@ static int hush_test_if_base(struct unit_test_state *uts) sprintf(if_formatted, if_format, "false"); ut_asserteq(1, run_command(if_formatted, 0)); + sprintf(if_formatted, if_format, "test"); + ut_asserteq(1, run_command(if_formatted, 0)); + + sprintf(if_formatted, if_format, "test ''"); + ut_asserteq(1, run_command(if_formatted, 0)); + + sprintf(if_formatted, if_format, "test 'abc'"); + ut_assertok(run_command(if_formatted, 0)); + return 0; } HUSH_TEST(hush_test_if_base, 0); @@ -343,6 +352,27 @@ static int hush_test_lbracket_alias(struct unit_test_state *uts) ut_asserteq(1, run_command(if_formatted, 0)); ut_assert_nextline(missing_rbracket_error); + sprintf(if_formatted, if_format, "[ ]"); + ut_asserteq(1, run_command(if_formatted, 0)); + + sprintf(if_formatted, if_format, "["); + ut_asserteq(1, run_command(if_formatted, 0)); + ut_assert_nextline(missing_rbracket_error); + + sprintf(if_formatted, if_format, "[ '' ]"); + ut_asserteq(1, run_command(if_formatted, 0)); + + sprintf(if_formatted, if_format, "[ ''"); + ut_asserteq(1, run_command(if_formatted, 0)); + ut_assert_nextline(missing_rbracket_error); + + sprintf(if_formatted, if_format, "[ 'abc' ]"); + ut_assertok(run_command(if_formatted, 0)); + + sprintf(if_formatted, if_format, "[ 'abc'"); + ut_asserteq(1, run_command(if_formatted, 0)); + ut_assert_nextline(missing_rbracket_error); + return 0; } HUSH_TEST(hush_test_lbracket_alias, UTF_CONSOLE); -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] cmd: test: fix handling of single-argument form of test 2026-03-11 12:09 ` [PATCH 3/3] cmd: test: fix handling of single-argument form of test Rasmus Villemoes @ 2026-03-16 2:43 ` Simon Glass 0 siblings, 0 replies; 12+ messages in thread From: Simon Glass @ 2026-03-16 2:43 UTC (permalink / raw) To: Rasmus Villemoes; +Cc: u-boot, Tom Rini Hi Rasmus, On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote: > > POSIX states that > > 0 arguments: > Exit false (1). > 1 argument: > Exit true (0) if $1 is not null; otherwise, exit false. > > and at least bash and busybox sh behave that way. > > The current 'argc < 3' does the right thing for a non-existing or > empty argv[1], but not for a non-empty argv[1]. Fix that and add > corresponding test cases. > > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > --- > cmd/test.c | 11 +++++++++-- > test/hush/if.c | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/cmd/test.c b/cmd/test.c > index f0645ef98f1..0d0f090386c 100644 > --- a/cmd/test.c > +++ b/cmd/test.c > @@ -71,10 +71,17 @@ static int do_test(struct cmd_tbl *cmdtp, int flag, int argc, > argc--; > } > > - /* args? */ > - if (argc < 3) > + /* > + * Per POSIX, 'test' with 0 arguments should return 1, while > + * 'test <arg>' should be equivalent to 'test -n <arg>', > + * i.e. true if and only if <arg> is not empty. > + */ > + if (argc < 2) > return 1; > > + if (argc == 2) > + return !strcmp(argv[1], ""); > + > #ifdef DEBUG > { > debug("test(%d):", argc); > diff --git a/test/hush/if.c b/test/hush/if.c > index 148e9a53e90..e82a95aaf27 100644 > --- a/test/hush/if.c > +++ b/test/hush/if.c > @@ -32,6 +32,15 @@ static int hush_test_if_base(struct unit_test_state *uts) > sprintf(if_formatted, if_format, "false"); > ut_asserteq(1, run_command(if_formatted, 0)); Can you use run_commandf() here? > > + sprintf(if_formatted, if_format, "test"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + > + sprintf(if_formatted, if_format, "test ''"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + > + sprintf(if_formatted, if_format, "test 'abc'"); > + ut_assertok(run_command(if_formatted, 0)); > + > return 0; > } > HUSH_TEST(hush_test_if_base, 0); > @@ -343,6 +352,27 @@ static int hush_test_lbracket_alias(struct unit_test_state *uts) > ut_asserteq(1, run_command(if_formatted, 0)); > ut_assert_nextline(missing_rbracket_error); > > + sprintf(if_formatted, if_format, "[ ]"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + > + sprintf(if_formatted, if_format, "["); > + ut_asserteq(1, run_command(if_formatted, 0)); > + ut_assert_nextline(missing_rbracket_error); > + > + sprintf(if_formatted, if_format, "[ '' ]"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + > + sprintf(if_formatted, if_format, "[ ''"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + ut_assert_nextline(missing_rbracket_error); > + > + sprintf(if_formatted, if_format, "[ 'abc' ]"); > + ut_assertok(run_command(if_formatted, 0)); > + > + sprintf(if_formatted, if_format, "[ 'abc'"); > + ut_asserteq(1, run_command(if_formatted, 0)); > + ut_assert_nextline(missing_rbracket_error); > + > return 0; > } > HUSH_TEST(hush_test_lbracket_alias, UTF_CONSOLE); > -- > 2.53.0 > Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-03-18 1:04 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-11 12:09 [PATCH 0/3] add [ as alias for test, fix 0/1 argument handling Rasmus Villemoes 2026-03-11 12:09 ` [PATCH 1/3] cmd: test: allow using [ as alias for test Rasmus Villemoes 2026-03-11 13:08 ` Quentin Schulz 2026-03-11 13:25 ` Rasmus Villemoes 2026-03-11 12:09 ` [PATCH 2/3] test: add tests for left-bracket alias for 'test' command Rasmus Villemoes 2026-03-16 2:43 ` Simon Glass 2026-03-16 8:16 ` Rasmus Villemoes 2026-03-17 12:28 ` Simon Glass 2026-03-17 15:11 ` Rasmus Villemoes 2026-03-18 1:03 ` Simon Glass 2026-03-11 12:09 ` [PATCH 3/3] cmd: test: fix handling of single-argument form of test Rasmus Villemoes 2026-03-16 2:43 ` Simon Glass
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox