6:24 p.m.
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
6:24 p.m.
New subject: [PATCH 1/3] networkcommon_conf: Use g_autofree where possible
Signed-off-by: Kristina Hanicova <khanicov(a)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
12:33 a.m.
New subject: [PATCH 1/3] networkcommon_conf: Use g_autofree where possible
On 2/23/21 12:24 PM, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov(a)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);
6:24 p.m.
New subject: [PATCH 2/3] Use g_autoptr instead of virNetDevIPRouteFree if possible
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(a)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
12:57 a.m.
New subject: [PATCH 2/3] Use g_autoptr instead of virNetDevIPRouteFree if possible
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(a)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(a)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);
6:24 p.m.
New subject: [PATCH 3/3] Remove redundant variables/labels
In files: src/lxc/lxc_native: in lxcAddNetworkRouteDefinition(),
src/conf/networkcommon_conf: in virNetDevIPRouteCreate() and
virNetDevIPRouteParseXML()
Signed-off-by: Kristina Hanicova <khanicov(a)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
1:29 a.m.
New subject: [PATCH 3/3] Remove redundant variables/labels
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(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)redhat.com>
1299
days inactive
1301
days old
6 comments
3 participants
participants (3)
-
Kristina Hanicova -
Laine Stump -
Laine Stump