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