Re: [PATCH 1/9] conf: use g_autofree and remove unnecessary label

On 1/5/23 6:26 AM, Jiang Jiacheng wrote: > Signed-off-by: Jiang Jiacheng <jiangjiacheng(a)huawei.com&gt; ... > diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c > index 9a95ae6c12..39f36ca29d 100644 > --- a/src/conf/nwfilter_conf.c > +++ b/src/conf/nwfilter_conf.c ... > @@ -2571,8 +2564,8 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) >       virNWFilterDef *ret; >       xmlNodePtr curr = ctxt->node; >       char *uuid = NULL; > -    char *chain = NULL; > -    char *chain_pri_s = NULL; > +    g_autofree char *chain = NULL; > +    g_autofree char *chain_pri_s = NULL; Is there a reason that you didn't use g_autofree for uuid here?

>       virNWFilterEntry *entry; >       int chain_priority; >       const char *name_prefix; > @@ -2671,16 +2664,11 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) >           curr = curr->next; >       } >   -    VIR_FREE(chain); > -    VIR_FREE(chain_pri_s); > - >       return ret; >      cleanup: >       virNWFilterDefFree(ret); > -    VIR_FREE(chain); >       VIR_FREE(uuid); > -    VIR_FREE(chain_pri_s); >       return NULL; >   } Would allow you to remove the free here. It might also be nice to add a followup commit to add an autofree function for virNWFilterDef so that we can remove this label (as well as the err_exit label in virNWFilterRuleParse()).

[...snip...] > diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c > index e8dfe66b3c..a8574beb79 100644 > --- a/src/conf/virnwfilterobj.c > +++ b/src/conf/virnwfilterobj.c > @@ -282,19 +282,15 @@ virNWFilterDefEqual(const virNWFilterDef *def1, >                       virNWFilterDef *def2) >   { >       bool ret = false; > -    char *xml1 = NULL; > -    char *xml2 = NULL; > +    g_autofree char *xml1 = NULL; > +    g_autofree char *xml2 = NULL; >         if (!(xml1 = virNWFilterDefFormat(def1)) || >           !(xml2 = virNWFilterDefFormat(def2))) > -        goto cleanup; > +        return false; >         ret = STREQ(xml1, xml2); >   - cleanup: > -    VIR_FREE(xml1); > -    VIR_FREE(xml2); > - >       return ret; >   } You should be able to remove the 'ret' variable and just return the result from STREQ >   @@ -573,7 +569,7 @@ virNWFilterObjListLoadConfig(virNWFilterObjList > *nwfilters, >   { >       virNWFilterDef *def = NULL; >       virNWFilterObj *obj; > -    char *configFile = NULL; > +    g_autofree char *configFile = NULL; >         if (!(configFile = virFileBuildPath(configDir, name, ".xml"))) >           goto error; > @@ -597,11 +593,9 @@ virNWFilterObjListLoadConfig(virNWFilterObjList > *nwfilters, >       if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) >           goto error; >   -    VIR_FREE(configFile); >       return obj; >      error: > -    VIR_FREE(configFile); >       virNWFilterDefFree(def); >       return NULL; >   } Here's another label that could be removed in a follow-up commit if you add an auto cleanup function for virNWFilterDefFree