Re: [PATCH for-5.1 5/8] qemu-option: Fix has_help_option()'s sloppy parsing

[Kevin Wolf <kwolf@redhat.com>]

Am 14.04.2020 um 12:16 hat Markus Armbruster geschrieben:
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 4/9/20 10:30 AM, Markus Armbruster wrote:
> >> has_help_option() uses its own parser.  It's inconsistent with
> >> qemu_opts_parse(), as demonstrated by test-qemu-opts case
> >> /qemu-opts/has_help_option.  Fix by reusing the common parser.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>

> >> @@ -165,26 +165,6 @@ void parse_option_size(const char *name, const char *value,
> >>       *ret = size;
> >>   }
> >>   -bool has_help_option(const char *param)
> >> -{
> >> -    const char *p = param;
> >> -    bool result = false;
> >> -
> >> -    while (*p && !result) {
> >> -        char *value;
> >> -
> >> -        p = get_opt_value(p, &value);
> >> -        if (*p) {
> >> -            p++;
> >> -        }
> >> -
> >> -        result = is_help_option(value);
> >
> > Old code: both 'help' and '?' are accepted.
> >
> >> +bool has_help_option(const char *params)
> >> +{
> >> +    const char *p;
> >> +    char *name, *value;
> >> +    bool ret;
> >> +
> >> +    for (p = params; *p;) {
> >> +        p = get_opt_name_value(p, NULL, &name, &value);
> >> +        ret = !strcmp(name, "help");
> >
> > New code: only 'help' is accepted.  Is the loss of '?' intentional?
> 
> No.  Will fix, thanks!

Please also add some '?' test cases while you're at it.

Kevin