[PATCH 0/3] Use g_autoptr and g_autofree
This uses g_autofree (if possible) in file networkcommon_conf and g_autoptr instead of virNetDevIPRouteFree where possible. Kristina Hanicova (3): networkcommon_conf: Use g_autofree where possible Use g_autoptr instead of virNetDevIPRouteFree if possible Remove redundant variables/labels src/conf/domain_conf.c | 3 +- src/conf/networkcommon_conf.c | 74 +++++++++++++++-------------------- src/lxc/lxc_native.c | 10 ++--- src/vz/vz_sdk.c | 3 +- 4 files changed, 36 insertions(+), 54 deletions(-) -- 2.29.2
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/networkcommon_conf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c index 26eeb6dbda..e82dbc3d3d 100644 --- a/src/conf/networkcommon_conf.c +++ b/src/conf/networkcommon_conf.c @@ -228,9 +228,10 @@ virNetDevIPRouteParseXML(const char *errorDetail, virNetDevIPRoutePtr def = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) - char *family = NULL; - char *address = NULL, *netmask = NULL; - char *gateway = NULL; + g_autofree char *family = NULL; + g_autofree char *address = NULL; + g_autofree char *netmask = NULL; + g_autofree char *gateway = NULL; unsigned long prefix = 0, metric = 0; int prefixRc, metricRc; bool hasPrefix = false; @@ -276,10 +277,6 @@ virNetDevIPRouteParseXML(const char *errorDetail, hasMetric); cleanup: - VIR_FREE(family); - VIR_FREE(address); - VIR_FREE(netmask); - VIR_FREE(gateway); return def; } @@ -287,7 +284,7 @@ int virNetDevIPRouteFormat(virBufferPtr buf, const virNetDevIPRoute *def) { - char *addr = NULL; + g_autofree char *addr = NULL; virBufferAddLit(buf, "<route"); @@ -311,7 +308,6 @@ virNetDevIPRouteFormat(virBufferPtr buf, if (!(addr = virSocketAddrFormat(&def->gateway))) return -1; virBufferAsprintf(buf, " gateway='%s'", addr); - VIR_FREE(addr); if (def->has_metric && def->metric > 0) virBufferAsprintf(buf, " metric='%u'", def->metric); -- 2.29.2
On 2/23/21 12:24 PM, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/networkcommon_conf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c index 26eeb6dbda..e82dbc3d3d 100644 --- a/src/conf/networkcommon_conf.c +++ b/src/conf/networkcommon_conf.c @@ -228,9 +228,10 @@ virNetDevIPRouteParseXML(const char *errorDetail,
virNetDevIPRoutePtr def = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) - char *family = NULL; - char *address = NULL, *netmask = NULL; - char *gateway = NULL; + g_autofree char *family = NULL; + g_autofree char *address = NULL; + g_autofree char *netmask = NULL; + g_autofree char *gateway = NULL; unsigned long prefix = 0, metric = 0; int prefixRc, metricRc; bool hasPrefix = false; @@ -276,10 +277,6 @@ virNetDevIPRouteParseXML(const char *errorDetail, hasMetric);
cleanup: - VIR_FREE(family); - VIR_FREE(address); - VIR_FREE(netmask); - VIR_FREE(gateway); return def; }
@@ -287,7 +284,7 @@ int virNetDevIPRouteFormat(virBufferPtr buf, const virNetDevIPRoute *def) { - char *addr = NULL; + g_autofree char *addr = NULL;
virBufferAddLit(buf, "<route");
@@ -311,7 +308,6 @@ virNetDevIPRouteFormat(virBufferPtr buf, if (!(addr = virSocketAddrFormat(&def->gateway))) return -1; virBufferAsprintf(buf, " gateway='%s'", addr); - VIR_FREE(addr);
This one is a bit more complicated, because "addr" is re-used after being freed (twice - first used for address, then for netmask, and finally for gateway), and we've made the rule that an auto-freed pointer should never be VIR_FREED So instead of just making the existing pointer g_autofree and eliminating its final free, we need to create separate pointers for each use: g_autofree char *address = NULL; g_autofree char *netmask = NULL; g_autofree char *gataeway = NULL; and then use each in the appropriate place. (if we're lucky, the compiler/optimizer will even be smart enough to figure out that the three things are never used at the same time (? depending on when the call to the autofree function is injected), and internally merge them into a single variable.)
if (def->has_metric && def->metric > 0) virBufferAsprintf(buf, " metric='%u'", def->metric);
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@redhat.com> --- 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) { - 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; 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); -- 2.29.2
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@redhat.com> --- 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@redhat.com> 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);
In files: src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(), src/conf/networkcommon_conf: in virNetDevIPRouteCreate() and virNetDevIPRouteParseXML() Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/networkcommon_conf.c | 55 +++++++++++++++-------------------- src/lxc/lxc_native.c | 7 ++--- 2 files changed, 26 insertions(+), 36 deletions(-) diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c index 9e7ad59337..a4c49b3caa 100644 --- a/src/conf/networkcommon_conf.c +++ b/src/conf/networkcommon_conf.c @@ -60,7 +60,7 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: Missing required address attribute " "in route definition"), errorDetail); - goto error; + return NULL; } if (!gateway) { @@ -68,7 +68,7 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: Missing required gateway attribute " "in route definition"), errorDetail); - goto error; + return NULL; } if (virSocketAddrParse(&def->address, address, AF_UNSPEC) < 0) { @@ -76,7 +76,7 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: Bad network address '%s' " "in route definition"), errorDetail, address); - goto error; + return NULL; } if (virSocketAddrParse(&def->gateway, gateway, AF_UNSPEC) < 0) { @@ -84,7 +84,7 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: Bad gateway address '%s' " "in route definition"), errorDetail, gateway); - goto error; + return NULL; } /* validate network address, etc. for each family */ @@ -98,7 +98,7 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: IPv4 family specified for non-IPv4 address '%s' " "in route definition"), errorDetail, address); - goto error; + return NULL; } if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET)) { virReportError(VIR_ERR_XML_ERROR, @@ -108,7 +108,7 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: IPv4 family specified for non-IPv4 gateway '%s' " "in route definition"), errorDetail, address); - goto error; + return NULL; } if (netmask) { if (virSocketAddrParse(&def->netmask, netmask, AF_UNSPEC) < 0) { @@ -116,14 +116,14 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: Bad netmask address '%s' " "in route definition"), errorDetail, netmask); - goto error; + return NULL; } if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->netmask, AF_INET)) { virReportError(VIR_ERR_XML_ERROR, _("%s: Invalid netmask '%s' " "for address '%s' (both must be IPv4)"), errorDetail, netmask, address); - goto error; + return NULL; } if (def->has_prefix) { /* can't have both netmask and prefix at the same time */ @@ -131,7 +131,7 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: Route definition cannot have both " "a prefix and a netmask"), errorDetail); - goto error; + return NULL; } } if (def->prefix > 32) { @@ -140,7 +140,7 @@ virNetDevIPRouteCreate(const char *errorDetail, "in route definition, " "must be 0 - 32"), errorDetail, def->prefix); - goto error; + return NULL; } } else if (STREQ(def->family, "ipv6")) { if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) { @@ -148,21 +148,21 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: ipv6 family specified for non-IPv6 address '%s' " "in route definition"), errorDetail, address); - goto error; + return NULL; } if (netmask) { virReportError(VIR_ERR_XML_ERROR, _("%s: Specifying netmask invalid for IPv6 address '%s' " "in route definition"), errorDetail, address); - goto error; + return NULL; } if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET6)) { virReportError(VIR_ERR_XML_ERROR, _("%s: ipv6 specified for non-IPv6 gateway address '%s' " "in route definition"), errorDetail, gateway); - goto error; + return NULL; } if (def->prefix > 128) { virReportError(VIR_ERR_XML_ERROR, @@ -170,14 +170,14 @@ virNetDevIPRouteCreate(const char *errorDetail, "in route definition, " "must be 0 - 128"), errorDetail, def->prefix); - goto error; + return NULL; } } else { virReportError(VIR_ERR_XML_ERROR, _("%s: Unrecognized family '%s' " "in route definition"), errorDetail, def->family); - goto error; + return NULL; } /* make sure the address is a network address */ @@ -188,7 +188,7 @@ virNetDevIPRouteCreate(const char *errorDetail, "to network-address " "in route definition"), errorDetail, address, netmask); - goto error; + return NULL; } } else { if (virSocketAddrMaskByPrefix(&def->address, @@ -198,7 +198,7 @@ virNetDevIPRouteCreate(const char *errorDetail, "to network-address " "in route definition"), errorDetail, address, def->prefix); - goto error; + return NULL; } } if (!virSocketAddrEqual(&def->address, &testAddr)) { @@ -206,13 +206,10 @@ virNetDevIPRouteCreate(const char *errorDetail, _("%s: Address '%s' in route definition " "is not a network address"), errorDetail, address); - goto error; + return NULL; } return g_steal_pointer(&def); - - error: - return NULL; } virNetDevIPRoutePtr @@ -225,7 +222,6 @@ virNetDevIPRouteParseXML(const char *errorDetail, * of an array. On failure clear: it out, but don't free it. */ - virNetDevIPRoutePtr def = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autofree char *family = NULL; g_autofree char *address = NULL; @@ -249,7 +245,7 @@ virNetDevIPRouteParseXML(const char *errorDetail, _("%s: Invalid prefix specified " "in route definition"), errorDetail); - goto cleanup; + return NULL; } hasPrefix = (prefixRc == 0); metricRc = virXPathULong("string(./@metric)", ctxt, &metric); @@ -258,7 +254,7 @@ virNetDevIPRouteParseXML(const char *errorDetail, _("%s: Invalid metric specified " "in route definition"), errorDetail); - goto cleanup; + return NULL; } if (metricRc == 0) { hasMetric = true; @@ -267,16 +263,13 @@ virNetDevIPRouteParseXML(const char *errorDetail, _("%s: Invalid metric value, must be > 0 " "in route definition"), errorDetail); - goto cleanup; + return NULL; } } - def = virNetDevIPRouteCreate(errorDetail, family, address, netmask, - gateway, prefix, hasPrefix, metric, - hasMetric); - - cleanup: - return def; + return virNetDevIPRouteCreate(errorDetail, family, address, netmask, + gateway, prefix, hasPrefix, metric, + hasMetric); } int diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index d5020dafa7..555e9acd89 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -452,15 +452,12 @@ lxcAddNetworkRouteDefinition(const char *address, if (!(route = virNetDevIPRouteCreate(_("Domain interface"), familyStr, zero, NULL, address, 0, false, 0, false))) - goto error; + return -1; if (VIR_APPEND_ELEMENT(*routes, *nroutes, route) < 0) - goto error; + return -1; return 0; - - error: - return -1; } static int -- 2.29.2
On 2/23/21 12:24 PM, Kristina Hanicova wrote:
In files: src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(), src/conf/networkcommon_conf: in virNetDevIPRouteCreate() and virNetDevIPRouteParseXML()
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
participants (3)
-
Kristina Hanicova -
Laine Stump -
Laine Stump