[PATCH 08/17] virXMLPropInt: Always initialize '@result'

Peter Krempa pkrempa at redhat.com
Thu May 6 17:14:26 UTC 2021


On Thu, May 06, 2021 at 19:07:59 +0200, Michal Prívozník wrote:
> On 5/6/21 5:31 PM, Peter Krempa wrote:
> > Compilers aren't able to see whether @result is set or not and thus
> > don't warn of a potential use of uninitialized value. Always set @result
> > to prevent uninitialized use.
> > 
> > This is done by adding a @defaultResult argument to virXMLPropInt since
> > many places have a non-0 default.
> > 
> > In certain cases such as in virDomainControllerDefParseXML we pass the
> > value from the original value, which will still trigger compiler checks
> > if unused while preserving the existing functionality of keeping the
> > previous value.
> > 
> > This commit fixes 3 uses of uninitialized value parsed by this function:
> >  in virDomainDiskSourceNetworkParse introduced by 38dc25989c5
> >  in virDomainChrSourceDefParseTCP introduced by fa48004af5b
> >  in virDomainGraphicsListenDefParseXML introduced by 0b20fd3754c
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >  src/conf/domain_conf.c | 57 +++++++++++++++++++++---------------------
> >  src/util/virxml.c      |  6 ++++-
> >  src/util/virxml.h      |  3 ++-
> >  3 files changed, 36 insertions(+), 30 deletions(-)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 78775bb2b3..2bc2e55ee4 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -8221,7 +8221,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> >      if (flags & VIR_DOMAIN_DEF_PARSE_STATUS) {
> >          int value;
> >          if (virXMLPropInt(node, "tlsFromConfig", 10, VIR_XML_PROP_NONE,
> > -                          &value) < 0)
> > +                          &value, 0) < 0)
> >              return -1;
> >          src->tlsFromConfig = !!value;
> >      }
> > @@ -9414,7 +9414,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
> >      g_autofree xmlNodePtr *modelNodes = NULL;
> >      int nmodelNodes = 0;
> >      int numaNode = -1;
> > -    int ports = -1;
> > +    int ports;
> >      VIR_XPATH_NODE_AUTORESTORE(ctxt)
> >      int rc;
> >      g_autofree char *idx = NULL;
> > @@ -9494,19 +9494,23 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
> >      if (ntargetNodes == 1) {
> >          if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
> >          if (virXMLPropInt(targetNodes[0], "chassisNr", 0, VIR_XML_PROP_NONE,
> > -                      &def->opts.pciopts.chassisNr) < 0)
> > +                      &def->opts.pciopts.chassisNr,
> > +                      def->opts.pciopts.chassisNr) < 0)
> 
> Ugrh, but I don't think there's much better option, unless we are
> willing to turn virXMLPropInt() into a macro. Something like:a

The other obvious option is to just populate it with the real values we
expect as default ('-1' in this case). I didn't want to declare them in
two places though.

In case virXMLPropInt it's a relatively large amount of cases when '0'
isn't the default thus I didn't opt for doing a version which would pick
0 as default as that would be almost pointless.




More information about the libvir-list mailing list