9:04 p.m.
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
9:04 p.m.
New subject: [PATCH 1/4] vsh-table: Use g_autofree where possible
In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout().
Signed-off-by: Kristina Hanicova <khanicov(a)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
8:20 p.m.
New subject: [PATCH 1/4] vsh-table: Use g_autofree where possible
On a Tuesday in 2021, Kristina Hanicova wrote:
In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout().
Signed-off-by: Kristina Hanicova <khanicov(a)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
8:36 p.m.
New subject: [PATCH 1/4] vsh-table: Use g_autofree where possible
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(a)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
11:54 p.m.
New subject: [PATCH 1/4] vsh-table: Use g_autofree where possible
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(a)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
7:25 a.m.
New subject: [PATCH 1/4] vsh-table: Use g_autofree where possible
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(a)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)
9:04 p.m.
New subject: [PATCH 2/4] netdev_bandwidth_conf: Use g_autofree where possible
Signed-off-by: Kristina Hanicova <khanicov(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
8:20 p.m.
New subject: [PATCH 2/4] netdev_bandwidth_conf: Use g_autofree where possible
On a Tuesday in 2021, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/netdev_bandwidth_conf.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:04 p.m.
New subject: [PATCH 3/4] Use g_autoptr instead of virNetDevBandwidthFree where possible
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(a)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
8:34 p.m.
New subject: [PATCH 3/4] Use g_autoptr instead of virNetDevBandwidthFree where possible
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(a)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;
}
9:04 p.m.
New subject: [PATCH 4/4] netdev_bandwidth_conf: Remove redundant variables/labels
In functions: virNetDevBandwidthParseRate(),
virNetDevBandwidthParse()
Signed-off-by: Kristina Hanicova <khanicov(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
8:34 p.m.
New subject: [PATCH 4/4] netdev_bandwidth_conf: Remove redundant variables/labels
On a Tuesday in 2021, Kristina Hanicova wrote:
In functions: virNetDevBandwidthParseRate(),
virNetDevBandwidthParse()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/netdev_bandwidth_conf.c | 42 +++++++++++++-------------------
1 file changed, 17 insertions(+), 25 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1284
days inactive
1292
days old
11 comments
4 participants
participants (4)
-
Ján Tomko -
Kristina Hanicova -
Laine Stump -
Michal Privoznik