[libvirt PATCH v3 16/51] virxml: Add virXMLPropTristateSwitch

[Daniel P. Berrangé]

On Mon, Mar 22, 2021 at 01:23:28PM +0100, Michal Privoznik wrote:
> On 3/19/21 4:57 PM, Tim Wiederhake wrote:
> > Convenience function to return value of an on / off attribute.
> > 
> > Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
> > ---
> >   src/libvirt_private.syms |  1 +
> >   src/util/virxml.c        | 41 ++++++++++++++++++++++++++++++++++++++++
> >   src/util/virxml.h        |  6 +++++-
> >   3 files changed, 47 insertions(+), 1 deletion(-)
> > 
> 
> > diff --git a/src/util/virxml.h b/src/util/virxml.h
> > index 3041c37df3..e844cb0713 100644
> > --- a/src/util/virxml.h
> > +++ b/src/util/virxml.h
> > @@ -80,8 +80,12 @@ char *     virXMLPropStringLimit(xmlNodePtr node,
> >   char *   virXMLNodeContentString(xmlNodePtr node);
> >   int       virXMLPropTristateBool(xmlNodePtr node,
> >                                    const char *name,
> > -                                 bool mandatory,
> > +                                 bool required,
> >                                    virTristateBool *result);
> 
> I guess this doesn't belong here. I'll squash it into the previous patch.

I wonder if we should avoid the boolean arg entirely though.
Looking at a call

  if (virXMLPropTristateBool(node, "foo", true, &result) < 0)

you have no idea whether "true" means mandatory or optional unless
you're very familiar with the code. In GTK/GNOME they generally
discourage use of bool parameters in favour of enums for this reason.
eg they would have a VIR_XML_TRISTATE_REQUIRED enum flag.

This is quite verbose though, so perhaps we can just use trivial
wrapper methods:

   virXMLPropTristateRequiredBool
   virXMLPropTristateOptionalBool

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 :|