On 2023/1/9 23:00, Ján Tomko wrote:
> On a Monday in 2023, Jiang Jiacheng wrote:
>> Use g_autoptr() for virNWFilterDef and virNWFilterRuleDef and remove
>> unnecessary label.
>>
>> Signed-off-by: Jiang Jiacheng <jiangjiach...@huawei.com>
>> ---
>> 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.
>
Thanks for your reply!
I will drop the two changes in the next version.
Thanks
Jiang Jiacheng
>>
>> 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;
