Re: [U-Boot] [PATCH v2 4/9] Stop using builtin_run_command()
Dear Simon Glass, In message 1323542641-14541-5-git-send-email-...@chromium.org you wrote: This function is only one of the parser options, so use run_command() instead. It knows how to use hush if selected. ... debug(Starting %s process...\n, __FUNCTION__); -#if !defined(CONFIG_SYS_HUSH_PARSER) - ret = builtin_run_command(s, 0); -#else - ret = parse_string_outer(s, FLAG_PARSE_SEMICOLON - | FLAG_EXIT_FROM_LOOP); -#endif + ret = run_command(s, 0); if (ret 0) debug(Error.. %s failed\n, __FUNCTION__); ... -#ifndef CONFIG_SYS_HUSH_PARSER - if (builtin_run_command(getenv(bootcmd), flag) 0) + if (run_command(getenv(bootcmd), flag) 0) rcode = 1; -#else - if (parse_string_outer(getenv(bootcmd), - FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0) - rcode = 1; -#endif I think this breaks error handling in a large scale. run_command2() in common/main.c (now renamed into run_command() will return 0 or 1 when used with the hush shell; it will never return 0. Please fix globally. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Parkinson's Law: Work expands to fill the time alloted it. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 4/9] Stop using builtin_run_command()
Hi Wolfgang, On Fri, Jan 13, 2012 at 11:27 AM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message 1323542641-14541-5-git-send-email-...@chromium.org you wrote: This function is only one of the parser options, so use run_command() instead. It knows how to use hush if selected. ... debug(Starting %s process...\n, __FUNCTION__); -#if !defined(CONFIG_SYS_HUSH_PARSER) - ret = builtin_run_command(s, 0); -#else - ret = parse_string_outer(s, FLAG_PARSE_SEMICOLON - | FLAG_EXIT_FROM_LOOP); -#endif + ret = run_command(s, 0); if (ret 0) debug(Error.. %s failed\n, __FUNCTION__); ... -#ifndef CONFIG_SYS_HUSH_PARSER - if (builtin_run_command(getenv(bootcmd), flag) 0) + if (run_command(getenv(bootcmd), flag) 0) rcode = 1; -#else - if (parse_string_outer(getenv(bootcmd), - FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0) - rcode = 1; -#endif I think this breaks error handling in a large scale. run_command2() in common/main.c (now renamed into run_command() will return 0 or 1 when used with the hush shell; it will never return 0. Thanks for looking at this. I will see what I can figure out. Regards, Simon Please fix globally. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Parkinson's Law: Work expands to fill the time alloted it. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 4/9] Stop using builtin_run_command()
Hi Wolfgang, On Fri, Jan 13, 2012 at 1:33 PM, Simon Glass s...@chromium.org wrote: Hi Wolfgang, On Fri, Jan 13, 2012 at 1:25 PM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message CAPnjgZ1=1+xjHoAbJd4WETjC6Bj+6NPgGsXbStoGadieznF1=q...@mail.gmail.com you wrote: run_command2() in common/main.c (now renamed into run_command() will return 0 or 1 when used with the hush shell; it will never return 0. Thanks for looking at this. I will see what I can figure out. I think there is some pre-existing inconsistency here, which pops up when you try to unify the code. I think we should implement standard shell handline here: commands return EXIT_SUCCESS or EXIT_FAILURE. I have had another look at this and simplified it a bit. Basically cmd_process() was the point of unification for command execution, so I have made that always return either 0 or 1. I think that should solve any problems. I will send out a v4 series. Regarding what commands return, I believe it is now true that commands are only called in one place. That place checks the return value for CMD_RET_USAGE. If it sees it, it calls cmd_usage() and returns 1. So CMD_RET_USAGE is never returned outside cmd_process(). It should just be a convenience for the command functions. In any case, all of the replacements of cmd_usage() with 'return CMD_RET_USAGE' are in the final patch if you want to leave that one out. I have tested with sandbox using hush and without. If there is still a problem please tell me how to repeat it, and I will take another look. Regards, Simon Regards, Simon Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de The most exciting phrase to hear in science, the one that heralds new discoveries, is not Eureka! (I found it!) but That's funny ... -- Isaac Asimov ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
