[U-Boot] [PATCH 1/4] common: add run_command2 for running simple or hush commands

[Jason Hobbs]

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