On Thu, Mar 24, 2022 at 12:42:12PM +0100, Peter Krempa wrote: > 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: > > > > 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. Yeah, I thought of that approach too and it sounds like it would be the least terrible one. I'm going to give it a try and report back :) Thanks for all your input so far! -- Andrea Bolognani / Red Hat / Virtualization