Re: [PATCH 2/3] Use g_autoptr instead of virNetDevIPRouteFree if possible

[Laine Stump <laine@xxxxxxxxx>]

On 2/23/21 12:24 PM, Kristina Hanicova wrote:
> In files: src/conf/domain_conf: in virDomainNetIPInfoParseXML(),
> src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(),
> src/vz/vz_sdk: in prlsdkGetRoutes(), src/conf/networkcommon_conf:
> in virNetDevIPRouteCreate()
>
> Signed-off-by: Kristina Hanicova <khanicov@xxxxxxxxxx>
> ---
>   src/conf/domain_conf.c        | 3 +--
>   src/conf/networkcommon_conf.c | 5 ++---
>   src/lxc/lxc_native.c          | 3 +--
>   src/vz/vz_sdk.c               | 3 +--
>   4 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b731744f04..2504911931 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7481,7 +7481,7 @@ virDomainNetIPInfoParseXML(const char *source,
>                              xmlXPathContextPtr ctxt,
>                              virNetDevIPInfoPtr def)

Not your problem, but I noticed that this function previously had the 
local "nodes" converted to g_autofree, apparently before the rule about 
"don't VIR_FREE an g_auto pointer" (because the change was made by the 
person who later called me on not following the rule :-))

Anyway, it wouldn't fit the theme to fix that in this patch, so it can 
be cleaned up later.

>   {
> -    virNetDevIPRoutePtr route = NULL;
> +    g_autoptr(virNetDevIPRoute) route = NULL;
>       int nnodes;
>       int ret = -1;
>       size_t i;
> @@ -7511,7 +7511,6 @@ virDomainNetIPInfoParseXML(const char *source,
>    cleanup:
>       if (ret < 0)
>           virNetDevIPInfoClear(def);
> -    virNetDevIPRouteFree(route);
>       return ret;
>   }
>   
> diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c
> index e82dbc3d3d..9e7ad59337 100644
> --- a/src/conf/networkcommon_conf.c
> +++ b/src/conf/networkcommon_conf.c
> @@ -41,7 +41,7 @@ virNetDevIPRouteCreate(const char *errorDetail,
>                          unsigned int metric,
>                          bool hasMetric)
>   {
> -    virNetDevIPRoutePtr def = NULL;
> +    g_autoptr(virNetDevIPRoute) def = NULL;
>       virSocketAddr testAddr;
>   
>       def = g_new0(virNetDevIPRoute, 1);
> @@ -209,10 +209,9 @@ virNetDevIPRouteCreate(const char *errorDetail,
>           goto error;
>       }
>   
> -    return def;
> +    return g_steal_pointer(&def);
>   
>    error:
> -    virNetDevIPRouteFree(def);
>       return NULL;
>   }
>   
> diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> index b903dc91d6..d5020dafa7 100644
> --- a/src/lxc/lxc_native.c
> +++ b/src/lxc/lxc_native.c
> @@ -441,7 +441,7 @@ lxcAddNetworkRouteDefinition(const char *address,
>                                virNetDevIPRoutePtr **routes,
>                                size_t *nroutes)
>   {
> -    virNetDevIPRoutePtr route = NULL;
> +    g_autoptr(virNetDevIPRoute) route = NULL;
>       g_autofree char *familyStr = NULL;
>       g_autofree char *zero = NULL;
>   
> @@ -460,7 +460,6 @@ lxcAddNetworkRouteDefinition(const char *address,
>       return 0;
>   
>    error:
> -    virNetDevIPRouteFree(route);
>       return -1;
>   }
>   
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index f49cd983f0..6f712c7a31 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -974,7 +974,7 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDefPtr net)
>       int ret = -1;
>       char *gw = NULL;
>       char *gw6 = NULL;
> -    virNetDevIPRoutePtr route = NULL;
> +    g_autoptr(virNetDevIPRoute) route = NULL;

Interesting. So this function uses route multiple times (once for IPv4 
and once for IPv6), but it's clearing out the first usage by stealing 
the pointer into and array of routes rather than freeing it. I guess 
that's okay, though.



Anyway:

Reviewed-by: Laine Stump <laine@xxxxxxxxxx>

I'll put this in a branch of things to push after the freeze is over.


>   
>       if (!(gw = prlsdkGetStringParamVar(PrlVmDevNet_GetDefaultGateway, sdknet)))
>           goto cleanup;
> @@ -1006,7 +1006,6 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDefPtr net)
>       ret = 0;
>   
>    cleanup:
> -    virNetDevIPRouteFree(route);
>       VIR_FREE(gw);
>       VIR_FREE(gw6);
>