Re: [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static

["Daniel P. Berrange" <berrange@redhat.com>]

On Fri, Sep 23, 2016 at 12:24:30PM +0200, Kevin Wolf wrote:
> Am 19.09.2016 um 13:58 hat Daniel P. Berrange geschrieben:
> > The opts-visitor.c opts_type_bool() method has code for
> > parsing a string to set a bool value, as does the
> > qemu-option.c parse_option_bool() method, except it
> > handles fewer cases.
> > 
> > To enable consistency across the codebase, extend
> > parse_option_bool() to handle "yes", "no", "y" and
> > "n", and make it non-static. Convert the opts
> > visitor to call this method directly.
> > 
> > Also make parse_option_number() non-static to allow
> > for similar reuse later.
> > 
> > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> 
> > --- a/util/qemu-option.c
> > +++ b/util/qemu-option.c
> > @@ -125,25 +125,30 @@ int get_param_value(char *buf, int buf_size,
> >      return get_next_param_value(buf, buf_size, tag, &str);
> >  }
> >  
> > -static void parse_option_bool(const char *name, const char *value, bool *ret,
> > -                              Error **errp)
> > +void parse_option_bool(const char *name, const char *value, bool *ret,
> > +                       Error **errp)
> >  {
> >      if (value != NULL) {
> > -        if (!strcmp(value, "on")) {
> > -            *ret = 1;
> > -        } else if (!strcmp(value, "off")) {
> > -            *ret = 0;
> > +        if (strcmp(value, "on") == 0 ||
> > +            strcmp(value, "yes") == 0 ||
> > +            strcmp(value, "y") == 0) {
> > +            *ret = true;
> > +        } else if (strcmp(value, "off") == 0 ||
> > +            strcmp(value, "no") == 0 ||
> > +            strcmp(value, "n") == 0) {
> > +            *ret = false;
> >          } else {
> > -            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> > -                       name, "'on' or 'off'");
> > +            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> > +                       "on|yes|y|off|no|n");
> 
> This change requires an update to the reference output of some
> qemu-iotests (at least 051 and 137).

Opps, yes, so it does.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|