On Thu, Mar 24, 2022 at 12:02:05 +0100, Martin Kletzander wrote:
> On Thu, Mar 24, 2022 at 10:32:51AM +0000, Andrea Bolognani wrote:
> > On Thu, Mar 24, 2022 at 09:58:28AM +0100, Martin Kletzander wrote:
> > > What I assume is that allowReboot is one of the few, if not the only
> > > exception where we format the default zero value.
> >
> > My guess is that you're right, but I haven't actually verified this
> > yet :)
> >
> > > > @@ -545,8 +545,6 @@ virXMLPropTristateBool(xmlNodePtr node,
> > > > virXMLPropFlags flags,
> > > > virTristateBool *result)
> > > > {
> > > > - flags |= VIR_XML_PROP_NONZERO;
> > > > -
> > >
> > > I would rather change this flag to something like
> > > VIR_XML_PROP_ALLOW_ZERO and only allow parsing default values with this
> > > flag making the callers be able to opt in for this behaviour rather then
> > > all the others having to opt out.
> >
> > Yeah, this sounds better from the caller's point of view, but it
> > would require adding a check to make sure that only one of
> > VIR_XML_PROP_NONZERO and VIR_XML_PROP_ALLOW_ZERO has been passed.
> > I'll see how clunky that looks.
> >
>
> I meant only keeping the new one, although I must admit I completely
> missed the fact that it is used properly somewhere else. If we went
> that way then the flag might itself be a tristate enum "default,
> allow_zero, non_zero" where default is different for numbers and enums,
> but that seems even clunkier and I don't like that myself.
Alternatively introduce a new helper e.g. virXMLPropTristateBoolDefault
which will allow also the 'default' value to be specified explicitly.
In such case we can document the difference in the comment and don't
have to touch every other usage.
