4:48 a.m.
This is v2 from
https://listman.redhat.com/archives/libvir-list/2021-February/msg01117.html
I split commit 3/4 into more commits and left out autofreeing outer
array as suggested by Jan Tomko.
Patches 2/6 and 6/6 (previously 2/4 and 4/4) have already been
reviewed-by and I am sending them for completness.
This uses g_autofree (if possible) in files: vsh-table,
netdev_bandwidth_conf and g_autoptr instead of
virNetDevBandwidthFree where possible.
Kristina Hanicova (6):
vsh-table: Use g_autofree where possible
netdev_bandwidth_conf: Use g_autofree where possible
netdev_bandwidth_conf: Refractor virNetDevBandwidthParse()
Use g_steal_pointer where possible
Use g_autoptr instead of virNetDevBandwidthFree where possible
netdev_bandwidth_conf: Remove redundant variables/labels
src/conf/netdev_bandwidth_conf.c | 67 ++++++++++++--------------------
src/network/bridge_driver.c | 6 +--
src/qemu/qemu_driver.c | 11 ++----
src/test/test_driver.c | 3 +-
src/util/virnetdevbandwidth.h | 2 +
tests/virnetdevbandwidthtest.c | 3 +-
tools/vsh-table.c | 13 ++-----
7 files changed, 39 insertions(+), 66 deletions(-)
--
2.29.2
4:48 a.m.
New subject: [PATCH v2 1/6] vsh-table: Use g_autofree where possible
In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout().
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
tools/vsh-table.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/tools/vsh-table.c b/tools/vsh-table.c
index d09cc9e14e..c5e57c0d7b 100644
--- a/tools/vsh-table.c
+++ b/tools/vsh-table.c
@@ -98,14 +98,12 @@ vshTableRowNew(const char *arg, va_list ap)
row = g_new0(vshTableRow, 1);
while (arg) {
- char *tmp = NULL;
+ g_autofree char *tmp = NULL;
tmp = g_strdup(arg);
- if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0) {
- VIR_FREE(tmp);
+ if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0)
goto error;
- }
arg = va_arg(ap, const char *);
}
@@ -361,7 +359,7 @@ vshTablePrint(vshTablePtr table, bool header)
{
size_t i;
size_t j;
- size_t *maxwidths;
+ g_autofree size_t *maxwidths = NULL;
size_t **widths;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
char *ret = NULL;
@@ -395,7 +393,6 @@ vshTablePrint(vshTablePtr table, bool header)
ret = virBufferContentAndReset(&buf);
cleanup:
- VIR_FREE(maxwidths);
for (i = 0; i < table->nrows; i++)
VIR_FREE(widths[i]);
VIR_FREE(widths);
@@ -416,15 +413,13 @@ void
vshTablePrintToStdout(vshTablePtr table, vshControl *ctl)
{
bool header;
- char *out;
+ g_autofree char *out = NULL;
header = ctl ? !ctl->quiet : true;
out = vshTablePrintToString(table, header);
if (out)
vshPrint(ctl, "%s", out);
-
- VIR_FREE(out);
}
--
2.29.2
4:48 a.m.
New subject: [PATCH v2 2/6] netdev_bandwidth_conf: Use g_autofree where possible
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/netdev_bandwidth_conf.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c
index 4fb7aa4e3d..1ff3785677 100644
--- a/src/conf/netdev_bandwidth_conf.c
+++ b/src/conf/netdev_bandwidth_conf.c
@@ -29,10 +29,10 @@ static int
virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr rate)
{
int ret = -1;
- char *average = NULL;
- char *peak = NULL;
- char *burst = NULL;
- char *floor = NULL;
+ g_autofree char *average = NULL;
+ g_autofree char *peak = NULL;
+ g_autofree char *burst = NULL;
+ g_autofree char *floor = NULL;
if (!node || !rate) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -88,11 +88,6 @@ virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr
rate)
ret = 0;
cleanup:
- VIR_FREE(average);
- VIR_FREE(peak);
- VIR_FREE(burst);
- VIR_FREE(floor);
-
return ret;
}
@@ -119,7 +114,7 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
virNetDevBandwidthPtr def = NULL;
xmlNodePtr cur;
xmlNodePtr in = NULL, out = NULL;
- char *class_id_prop = NULL;
+ g_autofree char *class_id_prop = NULL;
def = g_new0(virNetDevBandwidth, 1);
@@ -210,7 +205,6 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
ret = 0;
cleanup:
- VIR_FREE(class_id_prop);
virNetDevBandwidthFree(def);
return ret;
}
--
2.29.2
4:48 a.m.
New subject: [PATCH v2 3/6] netdev_bandwidth_conf: Refractor virNetDevBandwidthParse()
Refractoring includes:
* removal of VIR_FREE
* inversion of the condition
* use of g_steal_pointer
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/netdev_bandwidth_conf.c | 9 +++------
src/util/virnetdevbandwidth.h | 2 ++
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c
index 1ff3785677..81590efe6d 100644
--- a/src/conf/netdev_bandwidth_conf.c
+++ b/src/conf/netdev_bandwidth_conf.c
@@ -111,7 +111,7 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
bool allowFloor)
{
int ret = -1;
- virNetDevBandwidthPtr def = NULL;
+ g_autoptr(virNetDevBandwidth) def = NULL;
xmlNodePtr cur;
xmlNodePtr in = NULL, out = NULL;
g_autofree char *class_id_prop = NULL;
@@ -197,15 +197,12 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
}
}
- if (!def->in && !def->out)
- VIR_FREE(def);
+ if (def->in || def->out)
+ *bandwidth = g_steal_pointer(&def);
- *bandwidth = def;
- def = NULL;
ret = 0;
cleanup:
- virNetDevBandwidthFree(def);
return ret;
}
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
index 871d0c962c..fff83222a9 100644
--- a/src/util/virnetdevbandwidth.h
+++ b/src/util/virnetdevbandwidth.h
@@ -38,6 +38,8 @@ struct _virNetDevBandwidth {
void virNetDevBandwidthFree(virNetDevBandwidthPtr def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevBandwidth, virNetDevBandwidthFree);
+
int virNetDevBandwidthSet(const char *ifname,
const virNetDevBandwidth *bandwidth,
bool hierarchical_class,
--
2.29.2
10:33 a.m.
New subject: [PATCH v2 3/6] netdev_bandwidth_conf: Refractor virNetDevBandwidthParse()
On a Wednesday in 2021, Kristina Hanicova wrote:
Refractoring includes:
* removal of VIR_FREE
* inversion of the condition
* use of g_steal_pointer
The conversion to g_steal_pointer was in your patch:
Use g_steal_pointer where possible
and is already merged as
commit 155151a3d061e36417095f6fc09db3471a9eaa47
Jano
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/netdev_bandwidth_conf.c | 9 +++------
src/util/virnetdevbandwidth.h | 2 ++
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c
index 1ff3785677..81590efe6d 100644
--- a/src/conf/netdev_bandwidth_conf.c
+++ b/src/conf/netdev_bandwidth_conf.c
@@ -111,7 +111,7 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
bool allowFloor)
{
int ret = -1;
- virNetDevBandwidthPtr def = NULL;
+ g_autoptr(virNetDevBandwidth) def = NULL;
xmlNodePtr cur;
xmlNodePtr in = NULL, out = NULL;
g_autofree char *class_id_prop = NULL;
@@ -197,15 +197,12 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
}
}
- if (!def->in && !def->out)
- VIR_FREE(def);
+ if (def->in || def->out)
+ *bandwidth = g_steal_pointer(&def);
- *bandwidth = def;
- def = NULL;
ret = 0;
cleanup:
- virNetDevBandwidthFree(def);
return ret;
}
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
index 871d0c962c..fff83222a9 100644
--- a/src/util/virnetdevbandwidth.h
+++ b/src/util/virnetdevbandwidth.h
@@ -38,6 +38,8 @@ struct _virNetDevBandwidth {
void virNetDevBandwidthFree(virNetDevBandwidthPtr def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevBandwidth, virNetDevBandwidthFree);
+
int virNetDevBandwidthSet(const char *ifname,
const virNetDevBandwidth *bandwidth,
bool hierarchical_class,
--
2.29.2
4:48 a.m.
New subject: [PATCH v2 4/6] Use g_steal_pointer where possible
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/network/bridge_driver.c | 3 +--
src/qemu/qemu_driver.c | 6 ++----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 519a473995..29060139d7 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5466,8 +5466,7 @@ networkPortSetParameters(virNetworkPortPtr port,
goto cleanup;
virNetDevBandwidthFree(portdef->bandwidth);
- portdef->bandwidth = bandwidth;
- bandwidth = NULL;
+ portdef->bandwidth = g_steal_pointer(&bandwidth);
if (virNetworkPortDefSaveStatus(portdef, dir) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b9bbdf8d48..b18e46e383 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10375,8 +10375,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
virNetDevBandwidthFree(net->bandwidth);
if (newBandwidth->in || newBandwidth->out) {
- net->bandwidth = newBandwidth;
- newBandwidth = NULL;
+ net->bandwidth = g_steal_pointer(&newBandwidth);
} else {
net->bandwidth = NULL;
}
@@ -10394,8 +10393,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
if (persistentNet) {
if (!persistentNet->bandwidth) {
- persistentNet->bandwidth = bandwidth;
- bandwidth = NULL;
+ persistentNet->bandwidth = g_steal_pointer(&bandwidth);
} else {
if (bandwidth->in) {
VIR_FREE(persistentNet->bandwidth->in);
--
2.29.2
4:48 a.m.
New subject: [PATCH v2 5/6] Use g_autoptr instead of virNetDevBandwidthFree where possible
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/network/bridge_driver.c | 3 +--
src/qemu/qemu_driver.c | 5 ++---
src/test/test_driver.c | 3 +--
tests/virnetdevbandwidthtest.c | 3 +--
4 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 29060139d7..b29c37ef4c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5397,7 +5397,7 @@ networkPortSetParameters(virNetworkPortPtr port,
virNetworkObjPtr obj;
virNetworkDefPtr def;
virNetworkPortDefPtr portdef;
- virNetDevBandwidthPtr bandwidth = NULL;
+ g_autoptr(virNetDevBandwidth) bandwidth = NULL;
g_autofree char *dir = NULL;
int ret = -1;
size_t i;
@@ -5473,7 +5473,6 @@ networkPortSetParameters(virNetworkPortPtr port,
ret = 0;
cleanup:
- virNetDevBandwidthFree(bandwidth);
virNetworkObjEndAPI(&obj);
return ret;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b18e46e383..2a191232a6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10207,7 +10207,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
virDomainDefPtr persistentDef;
int ret = -1;
virDomainNetDefPtr net = NULL, persistentNet = NULL;
- virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL;
+ g_autoptr(virNetDevBandwidth) bandwidth = NULL;
+ g_autoptr(virNetDevBandwidth) newBandwidth = NULL;
g_autoptr(virQEMUDriverConfig) cfg = NULL;
bool inboundSpecified = false, outboundSpecified = false;
int actualType;
@@ -10421,8 +10422,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
qemuDomainObjEndJob(driver, vm);
cleanup:
- virNetDevBandwidthFree(bandwidth);
- virNetDevBandwidthFree(newBandwidth);
virDomainObjEndAPI(&vm);
return ret;
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index bca1297d1d..0ac03de127 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3446,7 +3446,7 @@ testDomainSetInterfaceParameters(virDomainPtr dom,
virDomainObjPtr vm = NULL;
virDomainDefPtr def;
virDomainNetDefPtr net = NULL;
- virNetDevBandwidthPtr bandwidth = NULL;
+ g_autoptr(virNetDevBandwidth) bandwidth = NULL;
bool inboundSpecified = false;
bool outboundSpecified = false;
size_t i;
@@ -3536,7 +3536,6 @@ testDomainSetInterfaceParameters(virDomainPtr dom,
ret = 0;
cleanup:
- virNetDevBandwidthFree(bandwidth);
virDomainObjEndAPI(&vm);
return ret;
}
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 5cbdd6ffea..2e76af3d0c 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -69,7 +69,7 @@ testVirNetDevBandwidthSet(const void *data)
int ret = -1;
const struct testSetStruct *info = data;
const char *iface = info->iface;
- virNetDevBandwidthPtr band = NULL;
+ g_autoptr(virNetDevBandwidth) band = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
char *actual_cmd = NULL;
@@ -98,7 +98,6 @@ testVirNetDevBandwidthSet(const void *data)
ret = 0;
cleanup:
virCommandSetDryRun(NULL, NULL, NULL);
- virNetDevBandwidthFree(band);
VIR_FREE(actual_cmd);
return ret;
}
--
2.29.2
4:48 a.m.
New subject: [PATCH v2 6/6] netdev_bandwidth_conf: Remove redundant variables/labels
In functions: virNetDevBandwidthParseRate(),
virNetDevBandwidthParse()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/netdev_bandwidth_conf.c | 42 +++++++++++++-------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c
index 81590efe6d..13abca94ec 100644
--- a/src/conf/netdev_bandwidth_conf.c
+++ b/src/conf/netdev_bandwidth_conf.c
@@ -28,7 +28,6 @@
static int
virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr rate)
{
- int ret = -1;
g_autofree char *average = NULL;
g_autofree char *peak = NULL;
g_autofree char *burst = NULL;
@@ -50,45 +49,42 @@ virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr
rate)
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("could not convert bandwidth average value
'%s'"),
average);
- goto cleanup;
+ return -1;
}
} else if (!floor) {
virReportError(VIR_ERR_XML_DETAIL, "%s",
_("Missing mandatory average or floor attributes"));
- goto cleanup;
+ return -1;
}
if ((peak || burst) && !average) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("'peak' and 'burst' require
'average' attribute"));
- goto cleanup;
+ return -1;
}
if (peak && virStrToLong_ullp(peak, NULL, 10, &rate->peak) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("could not convert bandwidth peak value
'%s'"),
peak);
- goto cleanup;
+ return -1;
}
if (burst && virStrToLong_ullp(burst, NULL, 10, &rate->burst) < 0)
{
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("could not convert bandwidth burst value
'%s'"),
burst);
- goto cleanup;
+ return -1;
}
if (floor && virStrToLong_ullp(floor, NULL, 10, &rate->floor) < 0)
{
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("could not convert bandwidth floor value
'%s'"),
floor);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- return ret;
+ return 0;
}
/**
@@ -110,7 +106,6 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
xmlNodePtr node,
bool allowFloor)
{
- int ret = -1;
g_autoptr(virNetDevBandwidth) def = NULL;
xmlNodePtr cur;
xmlNodePtr in = NULL, out = NULL;
@@ -121,7 +116,7 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
if (!node || !virXMLNodeNameEqual(node, "bandwidth")) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("invalid argument supplied"));
- goto cleanup;
+ return -1;
}
class_id_prop = virXMLPropString(node, "classID");
@@ -130,13 +125,13 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
virReportError(VIR_ERR_XML_DETAIL, "%s",
_("classID attribute not supported on <bandwidth>
"
"in this usage context"));
- goto cleanup;
+ return -1;
}
if (virStrToLong_ui(class_id_prop, NULL, 10, class_id) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse class id '%s'"),
class_id_prop);
- goto cleanup;
+ return -1;
}
}
@@ -149,7 +144,7 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
virReportError(VIR_ERR_XML_DETAIL, "%s",
_("Only one child <inbound> "
"element allowed"));
- goto cleanup;
+ return -1;
}
in = cur;
} else if (virXMLNodeNameEqual(cur, "outbound")) {
@@ -157,7 +152,7 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
virReportError(VIR_ERR_XML_DETAIL, "%s",
_("Only one child <outbound> "
"element allowed"));
- goto cleanup;
+ return -1;
}
out = cur;
}
@@ -171,13 +166,13 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
if (virNetDevBandwidthParseRate(in, def->in) < 0) {
/* helper reported error for us */
- goto cleanup;
+ return -1;
}
if (def->in->floor && !allowFloor) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("floor attribute is not supported for this
config"));
- goto cleanup;
+ return -1;
}
}
@@ -186,24 +181,21 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
if (virNetDevBandwidthParseRate(out, def->out) < 0) {
/* helper reported error for us */
- goto cleanup;
+ return -1;
}
if (def->out->floor) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("'floor' attribute allowed "
"only in <inbound> element"));
- goto cleanup;
+ return -1;
}
}
if (def->in || def->out)
*bandwidth = g_steal_pointer(&def);
- ret = 0;
-
- cleanup:
- return ret;
+ return 0;
}
static int
--
2.29.2
10:34 a.m.
On a Wednesday in 2021, Kristina Hanicova wrote:
This is v2 from
https://listman.redhat.com/archives/libvir-list/2021-February/msg01117.html
I split commit 3/4 into more commits and left out autofreeing outer
array as suggested by Jan Tomko.
Patches 2/6 and 6/6 (previously 2/4 and 4/4) have already been
reviewed-by and I am sending them for completness.
This uses g_autofree (if possible) in files: vsh-table,
netdev_bandwidth_conf and g_autoptr instead of
virNetDevBandwidthFree where possible.
Kristina Hanicova (6):
vsh-table: Use g_autofree where possible
netdev_bandwidth_conf: Use g_autofree where possible
netdev_bandwidth_conf: Refractor virNetDevBandwidthParse()
Use g_steal_pointer where possible
Use g_autoptr instead of virNetDevBandwidthFree where possible
netdev_bandwidth_conf: Remove redundant variables/labels
src/conf/netdev_bandwidth_conf.c | 67 ++++++++++++--------------------
src/network/bridge_driver.c | 6 +--
src/qemu/qemu_driver.c | 11 ++----
src/test/test_driver.c | 3 +-
src/util/virnetdevbandwidth.h | 2 +
tests/virnetdevbandwidthtest.c | 3 +-
tools/vsh-table.c | 13 ++-----
7 files changed, 39 insertions(+), 66 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
and pushed.
Jano
1359
days inactive
1360
days old
8 comments
2 participants
participants (2)
-
Ján Tomko -
Kristina Hanicova