Re: [PATCH 09/12] virSecurityDeviceLabelDefParseXML: Use automatic memory clearing for temp strings

On Mon, 2021-11-22 at 18:12 +0100, Peter Krempa wrote: > Apart from code simplification the refactor of 'model' fixes an > unlikely > memory leak of the string if a duplicate model is found. > > While the coversion of 'label' variable may seem unnecessary it will > come in handy in the next patch. > > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; > --- >  src/conf/domain_conf.c | 14 +++++++------- >  1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index bd9da0744d..e829511ac5 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -8016,7 +8016,10 @@ > virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDef > ***seclabels_rtn, >      size_t nseclabels = 0; >      int n; >      size_t i, j; > -    char *model, *relabel, *label, *labelskip; > +    g_autofree char *model = NULL; > +    g_autofree char *relabel = NULL; > +    g_autofree char *label = NULL; > +    g_autofree char *labelskip = NULL; >      g_autofree xmlNodePtr *list = NULL; > >      if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) < 0) > @@ -8041,7 +8044,7 @@ > virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDef > ***seclabels_rtn, >                      goto error; >                  } >              } > -            seclabels[i]->model = model; > +            seclabels[i]->model = g_steal_pointer(&model); >          } > >          relabel = virXMLPropString(list[i], "relabel"); > @@ -8050,10 +8053,8 @@ > virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDef > ***seclabels_rtn, >                  virReportError(VIR_ERR_XML_ERROR, >                                 _("invalid security relabel value > %s"), >                                 relabel); > -                VIR_FREE(relabel); >                  goto error; >              } > -            VIR_FREE(relabel); >          } else { >              seclabels[i]->relabel = true; >          } > @@ -8063,14 +8064,13 @@ > virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDef > ***seclabels_rtn, >          seclabels[i]->labelskip = false; >          if (labelskip && !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) >              ignore_value(virStringParseYesNo(labelskip, > &seclabels[i]->labelskip)); > -        VIR_FREE(labelskip); > >          ctxt->node = list[i]; >          label = virXPathStringLimit("string(./label)", >                                      VIR_SECURITY_LABEL_BUFLEN-1, > ctxt); > -        seclabels[i]->label = label; > +        seclabels[i]->label = g_steal_pointer(&label); > > -        if (label && !seclabels[i]->relabel) { > +        if (seclabels[i]->label && !seclabels[i]->relabel) { >              virReportError(VIR_ERR_XML_ERROR, >                             _("Cannot specify a label if relabelling > is " >                               "turned off. model=%s"), Theese look like they could be simplified to three calls to `virXMLPropTristateBool`