[libvirt PATCH 02/38] virxml: Add virXMLPropYesNo

[Daniel P. Berrangé]

On Thu, Mar 18, 2021 at 04:03:17PM +0100, Michal Privoznik wrote:
> On 3/18/21 9:00 AM, Tim Wiederhake wrote:
> > Convenience function to return value of a yes / no attribute.
> > 
> > Does not use virTristateBoolTypeFromString to disallow "default".
> > 
> > Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
> > ---
> >   src/util/virxml.c | 37 +++++++++++++++++++++++++++++++++++++
> >   src/util/virxml.h |  4 ++++
> >   2 files changed, 41 insertions(+)
> > 
> > diff --git a/src/util/virxml.c b/src/util/virxml.c
> > index 060b7530fc..47e8414bd5 100644
> > --- a/src/util/virxml.c
> > +++ b/src/util/virxml.c
> > @@ -556,6 +556,43 @@ virXMLNodeContentString(xmlNodePtr node)
> >   }
> > +/**
> > + * virXMLPropYesNo:
> > + * @node: XML dom node pointer
> > + * @name: Name of the property (attribute) to get
> > + * @value: The returned virTristateBool value
> > + *
> > + * Convenience function to return value of a yes / no attribute.
> > + *
> > + * Returns 1 in case of success in which case @value is set,
> > + *         or 0 if the attribute is not present,
> > + *         or -1 and reports an error on failure.
> > + */
> > +int
> > +virXMLPropYesNo(xmlNodePtr node, const char* name, virTristateBool *value)

I kind of feel this should be called

   virXMLPropTristateBool


and the next patch virXMLPropTristateSwitch, so that they
both match their output variable types.

> > +{
> > +    g_autofree char *tmp = virXMLPropString(node, name);
> > +
> > +    if (!tmp)
> > +        return 0;
> > +
> > +    if (STREQ("yes", tmp)) {
> > +        *value = VIR_TRISTATE_BOOL_YES;
> > +        return 1;
> > +    }
> > +
> > +    if (STREQ("no", tmp)) {
> > +        *value = VIR_TRISTATE_BOOL_NO;
> > +        return 1;
> > +    }
> > +
> > +    virReportError(VIR_ERR_XML_ERROR,
> > +                   _("Invalid value for attribute '%s' in node '%s': '%s'. Expected 'yes' or 'no'"),
> > +                   name, node->name, tmp);
> > +    return -1;
> 
> 
> How about:
> 
> int
> virXMLPropYesNo(xmlNodePtr node, const char* name, virTristateBool *value)
> {
>     g_autofree char *tmp = virXMLPropString(node, name);
>     int val;
> 
>     if (!tmp)
>         return 0;
> 
>     if ((val = virTristateBoolTypeFromString(tmp)) <= 0) {
>         virReportError(VIR_ERR_XML_ERROR,
>                        _("Invalid value for attribute '%s' in node '%s':
> '%s'. Expected 'yes' or 'no'"),
>                        name, node->name, tmp);
>         return -1;
>     }
> 
>     *value = val;
>     return 1;
> }

Yep, I think we really should do this, not open code the same
thing.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|