[PATCH 0/4] use g_autofree and g_autoptr
This uses g_autofree (if possible) in files: vsh-table, netdev_bandwidth_conf and g_autoptr instead of virNetDevBandwidthFree where possible. Kristina Hanicova (4): vsh-table: Use g_autofree where possible netdev_bandwidth_conf: Use g_autofree 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 | 16 +++----- 7 files changed, 40 insertions(+), 68 deletions(-) -- 2.29.2
In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout(). Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- tools/vsh-table.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tools/vsh-table.c b/tools/vsh-table.c index d09cc9e14e..2e10abfc90 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,8 +359,8 @@ vshTablePrint(vshTablePtr table, bool header) { size_t i; size_t j; - size_t *maxwidths; - size_t **widths; + g_autofree size_t *maxwidths = NULL; + g_autofree size_t **widths = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; char *ret = NULL; @@ -395,10 +393,8 @@ 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); return ret; } @@ -416,15 +412,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
On a Tuesday in 2021, Kristina Hanicova wrote:
In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout().
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- tools/vsh-table.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/tools/vsh-table.c b/tools/vsh-table.c index d09cc9e14e..2e10abfc90 100644 --- a/tools/vsh-table.c +++ b/tools/vsh-table.c @@ -361,8 +359,8 @@ vshTablePrint(vshTablePtr table, bool header) { size_t i; size_t j; - size_t *maxwidths; - size_t **widths; + g_autofree size_t *maxwidths = NULL; + g_autofree size_t **widths = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; char *ret = NULL;
@@ -395,10 +393,8 @@ 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);
While this does not change the behavior, mixing g_autofree for the outer array while using VIR_FREE for the per-row arrays feels incomplete. Please leave it out of the g_autofree conversion. Jano
return ret; }
@@ -416,15 +412,13 @@ void
On 2/25/21 1:20 PM, Ján Tomko wrote:
On a Tuesday in 2021, Kristina Hanicova wrote:
In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout().
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- tools/vsh-table.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/tools/vsh-table.c b/tools/vsh-table.c index d09cc9e14e..2e10abfc90 100644 --- a/tools/vsh-table.c +++ b/tools/vsh-table.c @@ -361,8 +359,8 @@ vshTablePrint(vshTablePtr table, bool header) { size_t i; size_t j; - size_t *maxwidths; - size_t **widths; + g_autofree size_t *maxwidths = NULL; + g_autofree size_t **widths = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; char *ret = NULL;
@@ -395,10 +393,8 @@ 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);
While this does not change the behavior, mixing g_autofree for the outer array while using VIR_FREE for the per-row arrays feels incomplete.
Any idea what would make it feel complete again? Michal
On a Thursday in 2021, Michal Privoznik wrote:
On 2/25/21 1:20 PM, Ján Tomko wrote:
On a Tuesday in 2021, Kristina Hanicova wrote:
In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout().
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- tools/vsh-table.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/tools/vsh-table.c b/tools/vsh-table.c index d09cc9e14e..2e10abfc90 100644 --- a/tools/vsh-table.c +++ b/tools/vsh-table.c @@ -361,8 +359,8 @@ vshTablePrint(vshTablePtr table, bool header) { size_t i; size_t j; - size_t *maxwidths; - size_t **widths; + g_autofree size_t *maxwidths = NULL; + g_autofree size_t **widths = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; char *ret = NULL;
@@ -395,10 +393,8 @@ 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);
While this does not change the behavior, mixing g_autofree for the outer array while using VIR_FREE for the per-row arrays feels incomplete.
Any idea what would make it feel complete again?
Freeing all the memory associated with 'widths' automatically, or none of it. So either: * introduce a new typedef for 'size_t **' and define a cleanup function for it that does that * use a different data type (does GLib have one that could do this for us?) But both seem out of scope of a simple g_autofree cleanup. Jano
Michal
On 3/1/21 10:54 AM, Ján Tomko wrote:
On a Thursday in 2021, Michal Privoznik wrote:
On 2/25/21 1:20 PM, Ján Tomko wrote:
On a Tuesday in 2021, Kristina Hanicova wrote:
In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout().
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- tools/vsh-table.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/tools/vsh-table.c b/tools/vsh-table.c index d09cc9e14e..2e10abfc90 100644 --- a/tools/vsh-table.c +++ b/tools/vsh-table.c @@ -361,8 +359,8 @@ vshTablePrint(vshTablePtr table, bool header) { size_t i; size_t j; - size_t *maxwidths; - size_t **widths; + g_autofree size_t *maxwidths = NULL; + g_autofree size_t **widths = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; char *ret = NULL;
@@ -395,10 +393,8 @@ 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);
While this does not change the behavior, mixing g_autofree for the outer array while using VIR_FREE for the per-row arrays feels incomplete.
Any idea what would make it feel complete again?
Freeing all the memory associated with 'widths' automatically, or none of it.
Yeah, I think that arrays of pointers to allocated objects were the only uses of VIR_FREE that I ended up not eliminating in my patches that removed *most* VIR_FREE from the esx directory. I didn't have the motivation to figure out the proper best way to fix them...
So either: * introduce a new typedef for 'size_t **' and define a cleanup function for it that does that * use a different data type (does GLib have one that could do this for us?)
glib has GArray (arrays of arbitrarily-sized elements) and GPtrArray (arrays of pointers). I suppose we should be using those (although they seem more complicated than VIR_INSERT_ELEMENT and friends, I'm probably unfairly biased. (I do acknowledge that it's nice that they have the size as a part of the array).
But both seem out of scope of a simple g_autofree cleanup.
Yeah, it's going to take some of that stuff, what's it called now?.... Oh yeah - initiative, that's what it is. (I'm actually just about to try fixing a bug (not related to memory management) using GArray for the first time. I'll let you know how it goes)
Signed-off-by: Kristina Hanicova <khanicov@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
On a Tuesday in 2021, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/netdev_bandwidth_conf.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
In files: netdev_bandwidth_conf: in virNetDevBandwidthParse(), bridge_driver: in networkPortSetParameters(), qemu_driver: in qemuDomainSetInterfaceParameters(), test_driver: in testDomainSetInterfaceParameters(), virnetdevbandwidthtest: in testVirNetDevBandwidthSet() Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/netdev_bandwidth_conf.c | 9 +++------ 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 +-- 6 files changed, 13 insertions(+), 21 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/network/bridge_driver.c b/src/network/bridge_driver.c index 519a473995..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; @@ -5466,15 +5466,13 @@ 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; ret = 0; cleanup: - virNetDevBandwidthFree(bandwidth); virNetworkObjEndAPI(&obj); return ret; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b9bbdf8d48..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; @@ -10375,8 +10376,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 +10394,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); @@ -10423,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/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, 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
On a Tuesday in 2021, Kristina Hanicova wrote:
In files: netdev_bandwidth_conf: in virNetDevBandwidthParse(), bridge_driver: in networkPortSetParameters(), qemu_driver: in qemuDomainSetInterfaceParameters(), test_driver: in testDomainSetInterfaceParameters(), virnetdevbandwidthtest: in testVirNetDevBandwidthSet()
For lists in commit messages, bullet lists are more readable: * virNetDevBandwidthParse * networkPortSetParameters Listing the changed files is not needed with git, usually people who see the commit message can see the diff list below (or use --stat):
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/netdev_bandwidth_conf.c | 9 +++------ 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 +-- 6 files changed, 13 insertions(+), 21 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;
This hunk combines: * removal of VIR_FREE * inversion of the condition * the g_steal_pointer change (which is also included in your other patch) For mass conversions across multiple functions or files, doing one type of change at a time is easier to read (and review). Alternatively, if a particular function needs more love, consider sending it as a separate "Refactor virNetDevBandwidthParse" commit.
cleanup: - virNetDevBandwidthFree(def); return ret; }
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 519a473995..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; @@ -5466,15 +5466,13 @@ networkPortSetParameters(virNetworkPortPtr port, goto cleanup;
virNetDevBandwidthFree(portdef->bandwidth);
- portdef->bandwidth = bandwidth; - bandwidth = NULL; + portdef->bandwidth = g_steal_pointer(&bandwidth);
These g_steal_pointer conversions are also not needed to use g_auto and can be separated. Jano
if (virNetworkPortDefSaveStatus(portdef, dir) < 0) goto cleanup;
ret = 0; cleanup: - virNetDevBandwidthFree(bandwidth); virNetworkObjEndAPI(&obj); return ret; }
In functions: virNetDevBandwidthParseRate(), virNetDevBandwidthParse() Signed-off-by: Kristina Hanicova <khanicov@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
On a Tuesday in 2021, Kristina Hanicova wrote:
In functions: virNetDevBandwidthParseRate(), virNetDevBandwidthParse()
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/netdev_bandwidth_conf.c | 42 +++++++++++++------------------- 1 file changed, 17 insertions(+), 25 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (4)
-
Ján Tomko -
Kristina Hanicova -
Laine Stump -
Michal Privoznik