[U-Boot] [PATCH v2 8/8] sandbox: Add basic command line parsing

[Simon Glass]

Hi Mike,

On Fri, Jan 20, 2012 at 11:05 AM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Tuesday 10 January 2012 19:45:52 Simon Glass wrote:
>> --- a/arch/sandbox/cpu/os.c
>> +++ b/arch/sandbox/cpu/os.c
>>
>> +static struct option long_options[] = {
>
> constify
>
Done

>> +void os_usage(int err)
>> +{
>> +     if (err < 0)
>> +             fprintf(stderr, "Try `--help' for more information.\n");
>> +     fprintf(stderr, "u-boot, "
>> +             "a command line test interface to U-Boot\n\n"
>> +             "usage:\tu-boot [-ch]\n"
>> +             "Options:\n"
>> +             "\t-h\tDisplay help\n"
>> +             "\t-c <command>\tExecute U-Boot command\n");
>> +     exit(1);
>> +}
>
> os_usage() should only write to stderr/exit(1) if it's an error, otherwise it
> should use stdout/exit(0)

OK, done. I knew there was a reason I wanted the error code.

>
>> +int os_parse_args(struct sandbox_state *state, int argc, char *argv[])
>> +{
>> ...
>> +     while ((c = getopt_long(argc, argv, "c:h",
>> +             long_options, NULL)) != EOF) {
>
> pull the optstring ("c:h") out of the getopt_long call and put it into a const
> next to the long_options[] variable
>
> and next to opts array

Much better thank you.

>
>> +     /* Execute command if required */
>> +     if (state->cmd) {
>> +             /* TODO: redo this when cmd tidy-up series lands */
>> +#ifdef CONFIG_SYS_HUSH_PARSER
>> +             run_command(state->cmd, 0);
>> +#else
>> +             parse_string_outer(state->cmd, FLAG_PARSE_SEMICOLON |
>> +                                 FLAG_EXIT_FROM_LOOP);
>> +#endif
>
> i'm not sure how useful -c is since we can already do:
>          ./u-boot <<<"some command"
>
> each to their own i guess ...

I had trouble getting that to work properly, ending up with a runaway
process. Probably we don't handle EOF correctly since we just loop
forever.

I do think this argument is useful and it (and help) provide a
reasonable excuse for a command line parser.

> -mike

Regards,
Simon