Hi, On 5 November 2014 13:11, Rabin Vincent <ra...@rab.in> wrote: > On Sat, Nov 01, 2014 at 09:12:37AM -0600, Simon Glass wrote: >> On 29 October 2014 16:21, Rabin Vincent <ra...@rab.in> 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