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

[Peter Krempa]

On Mon, Nov 22, 2021 at 21:02:01 +0100, Tim Wiederhake wrote:
> 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 at redhat.com>
> > ---
> >  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`

Indeed, 'relabel' and 'labelksip' can be parsed into a temp tristate and
then set the boolean in the label from it. I don't want to go further to
refactor all security labelling code to a real tristate.