[PATCH 3/4] Use g_autoptr instead of virNetDevBandwidthFree where possible

[Ján Tomko]

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 at 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;
> }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210225/9ab2d8d9/attachment-0001.sig>