Re: [U-Boot] [PATCH 6/6] hush: add some tests for quoting
Hi,
On 5 November 2014 13:11, Rabin Vincent wrote:
> On Sat, Nov 01, 2014 at 09:12:37AM -0600, Simon Glass wrote:
>> On 29 October 2014 16:21, Rabin Vincent wrote:
>> > + assert(run_command("setenv ut_var '\"'; setenv ut_var2
>> > \"${ut_var}\"", 0) == 0);
>> > + assert(!strcmp(getenv("ut_var2"), "\""));
>> > +
>> > + assert(run_command("setenv ut_catcat setenv ut_catout\\;setenv
>> > ut_catX setenv ut_catout \\\"\\$\\$ut_catin\\\"
>> > \\; run ut_catX", 0) == 0);
>> > + assert(run_command("setenv ut_pointer '1 2 3 4 5 6 7 8 9 10 11 12
>> > 13 14 15 16 17 18 19 20'", 0) == 0);
>>
>> Can we reduce this down to 3-4 numbers for easier maintenance? Or do
>> the 20 numbers buy us something?
>
> After 14 arguments, the quotes around them become necessary, so having
> more than 14 ensures we test that the quotes are still there:
>
> => setenv x 1 2 3 4 5 6 7 8 9 10 11 12 13
> => setenv x 1 2 3 4 5 6 7 8 9 10 11 12 13 14
> => setenv x 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
> setenv - set environment variables
OK sounds good. How about adding a test that CONFIG_SYS_MAXARGS is < you number.
Like
#if CONFIG_SYS_MAXARGS > 15
#error "..."
#endif
Otherwise if someone changes it in sandbox the test will change.
>
> Usage:
> setenv [-f] name value ...
> - [forcibly] set environment variable 'name' to 'value ...'
> setenv [-f] name
> - [forcibly] delete environment variable 'name'
> => setenv x '1 2 3 4 5 6 7 8 9 10 11 12 13 14 15'
> =>
>
>> Also did you test this with the simple cli parser too?
>
> No, I didn't realize that these tests they would get run without hush.
> I tried them out and they fail with the simple parser, as do the tests
> with the empty strings in the third patch. What do you suggest? Drop
> these new tests, or move them inside the ifdef HUSH?
#ifdef HUSH if the tests can't work without it.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 6/6] hush: add some tests for quoting
On Sat, Nov 01, 2014 at 09:12:37AM -0600, Simon Glass wrote:
> On 29 October 2014 16:21, Rabin Vincent wrote:
> > + assert(run_command("setenv ut_var '\"'; setenv ut_var2
> > \"${ut_var}\"", 0) == 0);
> > + assert(!strcmp(getenv("ut_var2"), "\""));
> > +
> > + assert(run_command("setenv ut_catcat setenv ut_catout\\;setenv
> > ut_catX setenv ut_catout \\\"\\$\\$ut_catin\\\" \\;
> > run ut_catX", 0) == 0);
> > + assert(run_command("setenv ut_pointer '1 2 3 4 5 6 7 8 9 10 11 12
> > 13 14 15 16 17 18 19 20'", 0) == 0);
>
> Can we reduce this down to 3-4 numbers for easier maintenance? Or do
> the 20 numbers buy us something?
After 14 arguments, the quotes around them become necessary, so having
more than 14 ensures we test that the quotes are still there:
=> setenv x 1 2 3 4 5 6 7 8 9 10 11 12 13
=> setenv x 1 2 3 4 5 6 7 8 9 10 11 12 13 14
=> setenv x 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
setenv - set environment variables
Usage:
setenv [-f] name value ...
- [forcibly] set environment variable 'name' to 'value ...'
setenv [-f] name
- [forcibly] delete environment variable 'name'
=> setenv x '1 2 3 4 5 6 7 8 9 10 11 12 13 14 15'
=>
> Also did you test this with the simple cli parser too?
No, I didn't realize that these tests they would get run without hush.
I tried them out and they fail with the simple parser, as do the tests
with the empty strings in the third patch. What do you suggest? Drop
these new tests, or move them inside the ifdef HUSH?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 6/6] hush: add some tests for quoting
Hi Rabin,
On 29 October 2014 16:21, Rabin Vincent wrote:
> Add a couple of tests for quoting. The indirect variable read test is
> a fixed version of the following expression from Fedora ARM's boot.scr.
> This unfixed expression stopped working after fe9ca3d ("hush: fix some
> quoted variable expansion issues"):
>
> setenv catcat setenv catout\;'setenv catX "setenv catout
> '\\\''\$\$catin'\\\''"' \; run catX
>
> Signed-off-by: Rabin Vincent
> ---
> test/command_ut.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/test/command_ut.c b/test/command_ut.c
> index 926573a..21804a4 100644
> --- a/test/command_ut.c
> +++ b/test/command_ut.c
> @@ -193,6 +193,16 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int
> argc, char * const argv[])
>
> assert(run_command("'", 0) == 1);
>
> + assert(run_command("setenv ut_var '\"'; setenv ut_var2
> \"${ut_var}\"", 0) == 0);
> + assert(!strcmp(getenv("ut_var2"), "\""));
> +
> + assert(run_command("setenv ut_catcat setenv ut_catout\\;setenv
> ut_catX setenv ut_catout \\\"\\$\\$ut_catin\\\" \\;
> run ut_catX", 0) == 0);
> + assert(run_command("setenv ut_pointer '1 2 3 4 5 6 7 8 9 10 11 12 13
> 14 15 16 17 18 19 20'", 0) == 0);
Can we reduce this down to 3-4 numbers for easier maintenance? Or do
the 20 numbers buy us something?
Also did you test this with the simple cli parser too?
> + assert(run_command("setenv ut_catin ut_pointer", 0) == 0);
> + assert(run_command("run ut_catcat", 0) == 0);
> + assert(getenv("ut_catout"));
> + assert(!strcmp(getenv("ut_catout"), "1 2 3 4 5 6 7 8 9 10 11 12 13 14
> 15 16 17 18 19 20"));
> +
> printf("%s: Everything went swimmingly\n", __func__);
> return 0;
> }
> --
> 2.1.1
>
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
