On Sun, 3 Jun 2012 12:48:48 +0100, Mark Walters <markwalters1009 at gmail.com> wrote: > > +static notmuch_bool_t > +_process_int_arg (const notmuch_opt_desc_t *arg_desc, char next, const char > *arg_str) { > + > + char *endptr; > + if (next == 0 || arg_str[0] == 0) { > + fprintf (stderr, "Option \"%s\" needs an integer argument.\n", > arg_desc->name); > + return FALSE; > + } > + > + *((int *)arg_desc->output_var) = strtol (arg_str, &endptr, 10); > + if (*endptr == 0) > + return TRUE;
It's usually clearer to write '\0' for the null character. > @@ -99,20 +133,13 @@ parse_option (const char *arg, > char next = arg[strlen (try->name)]; > const char *value= arg+strlen(try->name)+1; > > - char *endptr; > - > - /* Everything but boolean arguments (switches) needs a > - * delimiter, and a non-zero length value. Boolean > - * arguments may take an optional =true or =false value. > - */ > - if (next != '=' && next != ':' && next != 0) return FALSE; > - if (next == 0) { > - if (try->opt_type != NOTMUCH_OPT_BOOLEAN && > - try->opt_type != NOTMUCH_OPT_KEYWORD) > - return FALSE; > - } else { > - if (value[0] == 0) return FALSE; > - } > + /* If this is not the end of the argument (i.e. the next > + * character is not a space or a delimiter) we stop > + * parsing for this option but allow the parsing to > + * continue to for other options. This should allow > + * options to be initial segments of other options. */ It took me a little while to figure out what the last sentence was about. Perhaps: If we have not reached the end of the argument (i.e. the next character is not a space or delimiter) then the argument could still match a longer option name later in the option table. (otherwise, "continue to for other") > + if (next != '=' && next != ':' && next != 0) > + goto DONE_THIS_OPTION; The `goto' could be expressed as a `continue' in a `for' loop, AFAICS. > > if (try->output_var == NULL) > INTERNAL_ERROR ("output pointer NULL for option %s", try->name); > @@ -125,12 +152,10 @@ parse_option (const char *arg, > return _process_boolean_arg (try, next, value); > break; > case NOTMUCH_OPT_INT: > - *((int *)try->output_var) = strtol (value, &endptr, 10); > - return (*endptr == 0); > + return _process_int_arg (try, next, value); > break; > case NOTMUCH_OPT_STRING: > - *((const char **)try->output_var) = value; > - return TRUE; > + return _process_string_arg (try, next, value); > break; > case NOTMUCH_OPT_POSITION: > case NOTMUCH_OPT_END: > @@ -139,6 +164,7 @@ parse_option (const char *arg, > /*UNREACHED*/ > } > } > + DONE_THIS_OPTION: > try++; > } > fprintf (stderr, "Unrecognized option: --%s\n", arg); Peter