Re: [PATCH 2/3] src/tests: use g_autoptr for virNWFilterDef and virNWFilterRuleDef

On a Monday in 2023, Jiang Jiacheng wrote: > Use g_autoptr() for virNWFilterDef and virNWFilterRuleDef and remove > unnecessary label. > > Signed-off-by: Jiang Jiacheng <jiangjiacheng(a)huawei.com&gt; > --- > src/conf/nwfilter_conf.c       | 44 ++++++++++++++-------------------- > src/conf/virnwfilterobj.c      | 19 +++++++-------- > src/nwfilter/nwfilter_driver.c |  7 +++--- > tests/nwfilterxml2xmltest.c    | 22 +++++++---------- > 4 files changed, 37 insertions(+), 55 deletions(-) > > diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c > index 2e75e90cf1..2a2d877f49 100644 > --- a/src/conf/virnwfilterobj.c > +++ b/src/conf/virnwfilterobj.c > @@ -564,37 +564,34 @@ virNWFilterObjListLoadConfig(virNWFilterObjList > *nwfilters, >                              const char *configDir, >                              const char *name) > { > -    virNWFilterDef *def = NULL; > +    g_autoptr(virNWFilterDef) def = NULL; >     virNWFilterObj *obj; >     g_autofree char *configFile = NULL; > >     if (!(configFile = virFileBuildPath(configDir, name, ".xml"))) > -        goto error; > +        return NULL; > >     if (!(def = virNWFilterDefParse(NULL, configFile, 0))) > -        goto error; > +        return NULL; > >     if (STRNEQ(name, def->name)) { >         virReportError(VIR_ERR_XML_ERROR, >                        _("network filter config filename '%s' " >                          "does not match name '%s'"), >                        configFile, def->name); > -        goto error; > +        return NULL; >     } > >     /* We generated a UUID, make it permanent by saving the config to > disk */ >     if (!def->uuid_specified && >         virNWFilterSaveConfig(configDir, def) < 0) > -        goto error; > +        return NULL; > > -    if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) > -        goto error; > +    if (!(obj = virNWFilterObjListAssignDef(nwfilters, > +                                            g_steal_pointer(&def)))) > +        return NULL; Stealing the pointer here would lead to a memory leak if virNWFilterObjListAssignDef fails. The pointer can only be set to NULL after it was successfully assigned to the definition.

> >     return obj; > - > - error: > -    virNWFilterDefFree(def); > -    return NULL; > } > > > diff --git a/src/nwfilter/nwfilter_driver.c > b/src/nwfilter/nwfilter_driver.c > index 8e45096eaa..be21aa12c2 100644 > --- a/src/nwfilter/nwfilter_driver.c > +++ b/src/nwfilter/nwfilter_driver.c > @@ -531,7 +531,7 @@ nwfilterDefineXMLFlags(virConnectPtr conn, > @@ -552,10 +552,10 @@ nwfilterDefineXMLFlags(virConnectPtr conn, >         goto cleanup; > >     VIR_WITH_MUTEX_LOCK_GUARD(&driver->updateLock) { > -        if (!(obj = virNWFilterObjListAssignDef(driver->nwfilters, > def))) > +        if (!(obj = virNWFilterObjListAssignDef(driver->nwfilters, > +                                                g_steal_pointer(&def)))) >             goto cleanup; >     } > -    def = NULL; Same here. These two changes can be dropped. Jano >     objdef = virNWFilterObjGetDef(obj); > >     if (virNWFilterSaveConfig(driver->configDir, objdef) < 0) { > @@ -566,7 +566,6 @@ nwfilterDefineXMLFlags(virConnectPtr conn, >     nwfilter = virGetNWFilter(conn, objdef->name, objdef->uuid); > >  cleanup: > -    virNWFilterDefFree(def); >     if (obj) >         virNWFilterObjUnlock(obj); >     return nwfilter;