[libvirt PATCH v2 05/15] network: use g_auto wherever appropriate

[Ján Tomko]

On a Tuesday in 2020, Laine Stump wrote:
>This includes standard g_autofree() as well as other objects that have
>a cleanup function defined to use via g_autoptr (virCommand,
>virJSONValue)
>
>Signed-off-by: Laine Stump <laine at redhat.com>
>---
> src/network/bridge_driver.c       | 206 ++++++++++--------------------
> src/network/bridge_driver_linux.c |   7 +-
> src/network/leaseshelper.c        |  16 +--
> 3 files changed, 76 insertions(+), 153 deletions(-)
>
>diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>index ab359acdb5..31bd0dd92c 100644
>--- a/src/network/bridge_driver.c
>+++ b/src/network/bridge_driver.c

[...]

>@@ -1095,7 +1081,6 @@ networkDnsmasqConfContents(virNetworkObjPtr obj,
>     bool wantDNS = dns->enable != VIR_TRISTATE_BOOL_NO;
>     virNetworkIPDefPtr tmpipdef, ipdef, ipv4def, ipv6def;
>     bool ipv6SLAAC;
>-    char *saddr = NULL, *eaddr = NULL;
>
>     *configstr = NULL;
>

[...]

>@@ -1414,6 +1396,8 @@ networkDnsmasqConfContents(virNetworkObjPtr obj,
>             int thisRange;
>             virNetworkDHCPRangeDef range = ipdef->ranges[r];
>             g_autofree char *leasetime = NULL;
>+            g_autofree char *saddr = NULL;
>+            g_autofree char *eaddr = NULL;

300 lines below the original location. Long function is long. :)

>
>             if (!(saddr = virSocketAddrFormat(&range.addr.start)) ||
>                 !(eaddr = virSocketAddrFormat(&range.addr.end)))

[...]

>@@ -2248,7 +2206,7 @@ static int
> networkSetIPv6Sysctls(virNetworkObjPtr obj)
> {
>     virNetworkDefPtr def = virNetworkObjGetDef(obj);
>-    char *field = NULL;
>+    g_autofree char *field = NULL;

Last time I tried manually freeing an autofree'd variable, I was told
not to do that O:-)

The clean way here seems to be refactoring the function. I can put that
somewhere into the depths of my TODO list.

>     int ret = -1;
>     bool enableIPv6 = !!virNetworkDefGetIPByIndex(def, AF_INET6, 0);
>
>@@ -2273,7 +2231,6 @@ networkSetIPv6Sysctls(virNetworkObjPtr obj)
>                                "on bridge %s"), field, def->bridge);
>         goto cleanup;
>     }
>-    VIR_FREE(field);
>
>     /* The rest of the ipv6 sysctl tunables should always be set the
>      * same, whether or not we're using ipv6 on this bridge.
>@@ -2282,6 +2239,7 @@ networkSetIPv6Sysctls(virNetworkObjPtr obj)
>     /* Prevent guests from hijacking the host network by sending out
>      * their own router advertisements.
>      */
>+    VIR_FREE(field);
>     field = g_strdup_printf(SYSCTL_PATH "/net/ipv6/conf/%s/accept_ra",
>                             def->bridge);
>
>@@ -2290,11 +2248,11 @@ networkSetIPv6Sysctls(virNetworkObjPtr obj)
>                              _("cannot disable %s"), field);
>         goto cleanup;
>     }
>-    VIR_FREE(field);
>
>     /* All interfaces used as a gateway (which is what this is, by
>      * definition), must always have autoconf=0.
>      */
>+    VIR_FREE(field);
>     field = g_strdup_printf(SYSCTL_PATH "/net/ipv6/conf/%s/autoconf", def->bridge);
>
>     if (virFileWriteStr(field, "0", 0) < 0) {
>@@ -2305,7 +2263,6 @@ networkSetIPv6Sysctls(virNetworkObjPtr obj)
>
>     ret = 0;
>  cleanup:
>-    VIR_FREE(field);
>     return ret;
> }
>

[...]

>@@ -3276,8 +3221,6 @@ networkFindUnusedBridgeName(virNetworkObjListPtr nets,
>                    MAX_BRIDGE_ID);
>     ret = 0;

So this function returned 0 even on failure.
Introduced by a28d3e485f01d16320af15780bc935f54782a45d

>  cleanup:
>-    if (ret < 0)
>-        VIR_FREE(newname);
>     return ret;
> }
>

Without the networkSetIPv6Sysctls changes:
Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- 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/20200715/a8323c1c/attachment-0001.sig>