Fix a few problems with the command parsers
v2 cover Fix a few problems with the command parsers
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
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(-)
Signature · 1 line
1.9.1.423.g4596e3a
PATCH v2 1/4 Add final result tests for run_command_list()
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.
---
Changes in v2: None
test/command_ut.c | 5 +++++ 1 file changed, 5 insertions(+)
test/command_ut.c +5 -0
@@ -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 */
Signature · 1 line
1.9.1.423.g4596e3a
PATCH v2 1/4 Add final result tests for run_command_list()
Quoted 5 lines · Simon Glass
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 <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
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/3fb1e4a8/attachment.pgp>
PATCH v2 2/4 Fix itest mask overflow
In reply to Simon GlassThe mask value used in itest overflows and therefore it can return an incorrect result for something like 'itest 0 == 1'. Fix it.
---
Changes in v2: None
common/cmd_itest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
common/cmd_itest.c +1 -1
@@ -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)
Signature · 1 line
1.9.1.423.g4596e3a
PATCH v2 2/4 Fix itest mask overflow
Quoted 4 lines · Simon Glass
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 <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
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/34ca7608/attachment.pgp>
PATCH v2 3/4 Fix hush to give the correct return code for a simple command
In reply to Simon GlassWhen 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.
---
Changes in v2: None
common/cli_hush.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
common/cli_hush.c +3 -1
@@ -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
Signature · 1 line
1.9.1.423.g4596e3a
PATCH v2 3/4 Fix hush to give the correct return code for a simple command
Quoted 10 lines · Simon Glass
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_LOOPflag 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.
Quoted 7 lines
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 <[email protected]>Signed-off-by Simon Glass <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
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>
PATCH v2 4/4 Correct return code from builtin_run_command_list()
In reply to Simon GlassThe 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.
---
Changes in v2:
- Rebase to master
common/cli_simple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
common/cli_simple.c +1 -1
@@ -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; }
Signature · 1 line
1.9.1.423.g4596e3a
PATCH v2 4/4 Correct return code from builtin_run_command_list()
Quoted 5 lines · Simon Glass
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 <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
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/8057a867/attachment.pgp>
v2 cover Fix a few problems with the command parsers
In reply to Simon GlassHi Tom,
Quoted 8 lines · Simon Glass
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.
Quoted 7 lines
Simon Glass (4):
Add final result tests forrun_command_list()
Fix itest mask overflow
Fix hush to give the correct return code for a simple command Correct return code frombuiltin_run_command_list()
Quoted 9 lines
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