Hi Mike, On Fri, Jan 20, 2012 at 11:05 AM, Mike Frysinger <vap...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot