* [U-Boot] [PATCH v2 1/4] Add final result tests for run_command_list()
2014-05-30 20:41 [U-Boot] [PATCH v2 0/4] Fix a few problems with the command parsers Simon Glass
@ 2014-05-30 20:41 ` Simon Glass
2014-06-05 22:47 ` [U-Boot] [U-Boot, v2, " Tom Rini
2014-05-30 20:41 ` [U-Boot] [PATCH v2 2/4] Fix itest mask overflow Simon Glass
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2014-05-30 20:41 UTC (permalink / raw)
To: u-boot
run_command_list() is supposed to return a return code of 0 for success
and 1 for failure. Add a few simple tests that confirm this. These tests
work both with the built-in parser and hush.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
test/command_ut.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/test/command_ut.c b/test/command_ut.c
index aaa1ee2..b2666bf 100644
--- a/test/command_ut.c
+++ b/test/command_ut.c
@@ -61,6 +61,11 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
"setenv list ${list}3", strlen("setenv list 1"), 0);
assert(!strcmp("1", getenv("list")));
+ assert(run_command("false", 0) == 1);
+ assert(run_command("echo", 0) == 0);
+ assert(run_command_list("false", -1, 0) == 1);
+ assert(run_command_list("echo", -1, 0) == 0);
+
#ifdef CONFIG_SYS_HUSH_PARSER
/* Test the 'test' command */
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 2/4] Fix itest mask overflow
2014-05-30 20:41 [U-Boot] [PATCH v2 0/4] Fix a few problems with the command parsers Simon Glass
2014-05-30 20:41 ` [U-Boot] [PATCH v2 1/4] Add final result tests for run_command_list() Simon Glass
@ 2014-05-30 20:41 ` Simon Glass
2014-06-05 22:47 ` [U-Boot] [U-Boot,v2,2/4] " Tom Rini
2014-05-30 20:41 ` [U-Boot] [PATCH v2 3/4] Fix hush to give the correct return code for a simple command Simon Glass
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2014-05-30 20:41 UTC (permalink / raw)
To: u-boot
The mask value used in itest overflows and therefore it can return an
incorrect result for something like 'itest 0 == 1'. Fix it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
common/cmd_itest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_itest.c b/common/cmd_itest.c
index ae2527b..76af62b 100644
--- a/common/cmd_itest.c
+++ b/common/cmd_itest.c
@@ -63,7 +63,7 @@ static long evalexp(char *s, int w)
l = simple_strtoul(s, NULL, 16);
}
- return (l & ((1 << (w * 8)) - 1));
+ return l & ((1UL << (w * 8)) - 1);
}
static char * evalstr(char *s)
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 3/4] Fix hush to give the correct return code for a simple command
2014-05-30 20:41 [U-Boot] [PATCH v2 0/4] Fix a few problems with the command parsers Simon Glass
2014-05-30 20:41 ` [U-Boot] [PATCH v2 1/4] Add final result tests for run_command_list() Simon Glass
2014-05-30 20:41 ` [U-Boot] [PATCH v2 2/4] Fix itest mask overflow Simon Glass
@ 2014-05-30 20:41 ` Simon Glass
2014-06-05 22:47 ` [U-Boot] [U-Boot, v2, " Tom Rini
2014-05-30 20:41 ` [U-Boot] [PATCH v2 4/4] Correct return code from builtin_run_command_list() Simon Glass
2014-05-30 20:43 ` [U-Boot] [PATCH v2 0/4] Fix a few problems with the command parsers Simon Glass
4 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2014-05-30 20:41 UTC (permalink / raw)
To: u-boot
When a simple command like 'false' is provided, hush should return the
result of that command. However, hush only does this if the
FLAG_EXIT_FROM_LOOP flag is provided. Without this flag, hush will
happily execute the empty string command immediate after 'false' and
then return a success code.
This behaviour does not seem very useful, and requiring the flag also
seems wrong, since it means that hush will execute only the first command
in a sequence.
Add a check for empty string and fall out of the loop in that case. That
at least fixes the simple command case. This is a change in behaviour but
it is unlikely that the old behaviour would be considered correct in any
case.
Reported-by: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
common/cli_hush.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/cli_hush.c b/common/cli_hush.c
index 0f069b0..e0c436f 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -3215,7 +3215,9 @@ static int parse_stream_outer(struct in_str *inp, int flag)
free_pipe_list(ctx.list_head,0);
}
b_free(&temp);
- } while (rcode != -1 && !(flag & FLAG_EXIT_FROM_LOOP)); /* loop on syntax errors, return on EOF */
+ /* loop on syntax errors, return on EOF */
+ } while (rcode != -1 && !(flag & FLAG_EXIT_FROM_LOOP) &&
+ (inp->peek != static_peek || b_peek(inp)));
#ifndef __U_BOOT__
return 0;
#else
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [U-Boot, v2, 3/4] Fix hush to give the correct return code for a simple command
2014-05-30 20:41 ` [U-Boot] [PATCH v2 3/4] Fix hush to give the correct return code for a simple command Simon Glass
@ 2014-06-05 22:47 ` Tom Rini
0 siblings, 0 replies; 10+ messages in thread
From: Tom Rini @ 2014-06-05 22:47 UTC (permalink / raw)
To: u-boot
On Fri, May 30, 2014 at 02:41:50PM -0600, Simon Glass wrote:
> When a simple command like 'false' is provided, hush should return the
> result of that command. However, hush only does this if the
> FLAG_EXIT_FROM_LOOP flag is provided. Without this flag, hush will
> happily execute the empty string command immediate after 'false' and
> then return a success code.
>
> This behaviour does not seem very useful, and requiring the flag also
> seems wrong, since it means that hush will execute only the first command
> in a sequence.
>
> Add a check for empty string and fall out of the loop in that case. That
> at least fixes the simple command case. This is a change in behaviour but
> it is unlikely that the old behaviour would be considered correct in any
> case.
>
> Reported-by: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net>
> Signed-off-by: Simon Glass <sjg@chromium.org>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140605/2613f809/attachment.pgp>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 4/4] Correct return code from builtin_run_command_list()
2014-05-30 20:41 [U-Boot] [PATCH v2 0/4] Fix a few problems with the command parsers Simon Glass
` (2 preceding siblings ...)
2014-05-30 20:41 ` [U-Boot] [PATCH v2 3/4] Fix hush to give the correct return code for a simple command Simon Glass
@ 2014-05-30 20:41 ` Simon Glass
2014-06-05 22:48 ` [U-Boot] [U-Boot, v2, " Tom Rini
2014-05-30 20:43 ` [U-Boot] [PATCH v2 0/4] Fix a few problems with the command parsers Simon Glass
4 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2014-05-30 20:41 UTC (permalink / raw)
To: u-boot
The return code is not consistent with cli_simple_run_command_list(). For the
last command in a sequence, the return code is actually inverted.
Fix it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Rebase to master
common/cli_simple.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cli_simple.c b/common/cli_simple.c
index 413c2eb..49d5833 100644
--- a/common/cli_simple.c
+++ b/common/cli_simple.c
@@ -331,7 +331,7 @@ int cli_simple_run_command_list(char *cmd, int flag)
++next;
}
if (rcode == 0 && *line)
- rcode = (cli_simple_run_command(line, 0) >= 0);
+ rcode = (cli_simple_run_command(line, 0) < 0);
return rcode;
}
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH v2 0/4] Fix a few problems with the command parsers
2014-05-30 20:41 [U-Boot] [PATCH v2 0/4] Fix a few problems with the command parsers Simon Glass
` (3 preceding siblings ...)
2014-05-30 20:41 ` [U-Boot] [PATCH v2 4/4] Correct return code from builtin_run_command_list() Simon Glass
@ 2014-05-30 20:43 ` Simon Glass
4 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2014-05-30 20:43 UTC (permalink / raw)
To: u-boot
Hi Tom,
On 30 May 2014 14:41, Simon Glass <sjg@chromium.org> wrote:
>
> In a few cases the behaviour of both the hush and built-in parsers seems
> incorrect. One such case was exposed by commit 1992dbf which attempted to
> execute a simple command using hush and get the correct return value.
> Further digging exposed the other problems.
>
> Changes in v2:
> - Rebase to master
Here is the rebase, but note I have at least one more thing to look at
for this release.
>
> Simon Glass (4):
> Add final result tests for run_command_list()
> Fix itest mask overflow
> Fix hush to give the correct return code for a simple command
> Correct return code from builtin_run_command_list()
>
> common/cli_hush.c | 4 +++-
> common/cli_simple.c | 2 +-
> common/cmd_itest.c | 2 +-
> test/command_ut.c | 5 +++++
> 4 files changed, 10 insertions(+), 3 deletions(-)
>
> --
> 1.9.1.423.g4596e3a
>
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread