On Thu, Sep 02, 2021 at 05:11:03PM +0200, Ján Tomko wrote:
> On a Thursday in 2021, Kristina Hanicova wrote:
> > Signed-off-by: Kristina Hanicova <khani...@redhat.com>
> > ---
> > src/conf/network_conf.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> > index f23599abac..5e24880f1f 100644
> > --- a/src/conf/network_conf.c
> > +++ b/src/conf/network_conf.c
> > @@ -2093,7 +2093,8 @@ virNetworkDefParse(const char *xmlStr,
> >     int keepBlanksDefault = xmlKeepBlanksDefault(0);
> > 
> >     if ((xml = virXMLParse(filename, xmlStr, _("(network_definition)"),
> > -                           "network.rng", flags & 
> > VIR_NETWORK_DEFINE_VALIDATE)))
> > +                           "network.rng", flags & 
> > (VIR_NETWORK_DEFINE_VALIDATE |
> > +                                                   
> > VIR_NETWORK_CREATE_VALIDATE))))
> >         def = virNetworkDefParseNode(xml, xmlDocGetRootElement(xml), 
> > xmlopt);
> > 
> 
> This has no functional effect, since 1 | 1 == 1.
> 
> I think leaving only one symbolic representation of 1 here is less
> confusing.

Clearly the problem is that we are passing in the "flags" param
from 2 different public APIs into the same internal method and
just blindly assuming all flags are equivalent. This is only
working by luck.

It is probably saner if the internal method has a "bool validate"
parameter and didn't use the public flags ?

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

Reply via email to