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

[Jiang Jiacheng]

On 2023/1/6 1:33, Jonathon Jongsma wrote:
> On 1/5/23 6:26 AM, Jiang Jiacheng wrote:
>> Signed-off-by: Jiang Jiacheng <jiangjiacheng at huawei.com>
> 
> ...
> 
>> 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?

It seems like could be used with g_autofree, I will modify it in next
version.

> 
>>       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()).
> 
> 

I'm glad to do so. I will try to use g_autoptr() for them in a
followup commit.

> 
> [...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
>