Dear Wolfgang,
On Mon, Jun 06, 2011 at 09:16:00PM +0200, Wolfgang Denk wrote:
-# ifndef CONFIG_SYS_HUSH_PARSER
run_command (p, 0);-# else
parse_string_outer(p, FLAG_PARSE_SEMICOLON |FLAG_EXIT_FROM_LOOP);-# endif
- run_command2(p, 0);
Indentation looks incorrect here?
Yes, it is incorrect. I will correct it in the next version of the patch.
-# endif
}
if (s)run_command2(s, 0);Indentation always by TABs please.
The if (s) line and the rest of this block were indented with spaces before I changed it. Should I make the cosmetic change of correct the indentation there to use tabs as part of this patch?
+int run_command2(const char *cmd, int flag) +{ +#ifndef CONFIG_SYS_HUSH_PARSER
- if (run_command(cmd, flag) == -1)
return 1;+#else
- if (parse_string_outer(cmd,
FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0)return 1;+#endif
- return 0;
Why do you make this function return int when there is no tesing of the return code anywhere?
Where run_command2 replaced run_command/parse_string_outer blocks in main.c, there was no checking of the return codes of those functions already. I do use the return code of it in cmd_pxecfg.c, which is introduced later in this patch series.
And why do you make this not an inline function (or macro) so the compiler could avoid the function call overhead?
No particular reason - I just didn't consider optimization. Is common.h the best place to place this as an inline function?
Thanks, Jason