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

[Jonathon Jongsma <jjongsma@xxxxxxxxxx>]

On 1/5/23 6:26 AM, Jiang Jiacheng wrote:
> Signed-off-by: Jiang Jiacheng <jiangjiacheng@xxxxxxxxxx>

...

> 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