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

Reply via email to