[PATCH] network: refactor networkSetIPv6Sysctls() for proper g_autofree usage

Laine Stump posted 1 patch 3 years, 9 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200718033346.816154-1-laine@redhat.com
src/network/bridge_driver.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
[PATCH] network: refactor networkSetIPv6Sysctls() for proper g_autofree usage
Posted by Laine Stump 3 years, 9 months ago
This function used the same char* three times for different purposes,
freeing it after each use. Since we don't want to ever manually free
an autofree'd pointer, modify it to use three separate char*, and make
them all g_autofree.

Signed-off-by: Laine Stump <laine@redhat.com>
---
This was suggested by Jan in

  https://www.redhat.com/archives/libvir-list/2020-July/msg00805.html

pushing this patch along with the patch 5 referenced there will permit
pushing patch 06/15 of that series unmodified.

 src/network/bridge_driver.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index dd8f34e543..6d341dba7c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2248,7 +2248,9 @@ static int
 networkSetIPv6Sysctls(virNetworkObjPtr obj)
 {
     virNetworkDefPtr def = virNetworkObjGetDef(obj);
-    char *field = NULL;
+    g_autofree char *disable_ipv6 = NULL;
+    g_autofree char *accept_ra = NULL;
+    g_autofree char *autoconf = NULL;
     int ret = -1;
     bool enableIPv6 = !!virNetworkDefGetIPByIndex(def, AF_INET6, 0);
 
@@ -2256,10 +2258,10 @@ networkSetIPv6Sysctls(virNetworkObjPtr obj)
      * network. But also unset it if there *are* ipv6 addresses, as we
      * can't be sure of its default value.
      */
-    field = g_strdup_printf(SYSCTL_PATH "/net/ipv6/conf/%s/disable_ipv6",
-                            def->bridge);
+    disable_ipv6 = g_strdup_printf(SYSCTL_PATH "/net/ipv6/conf/%s/disable_ipv6",
+                                   def->bridge);
 
-    if (access(field, W_OK) < 0 && errno == ENOENT) {
+    if (access(disable_ipv6, W_OK) < 0 && errno == ENOENT) {
         if (!enableIPv6)
             VIR_DEBUG("ipv6 appears to already be disabled on %s",
                       def->bridge);
@@ -2267,13 +2269,12 @@ networkSetIPv6Sysctls(virNetworkObjPtr obj)
         goto cleanup;
     }
 
-    if (virFileWriteStr(field, enableIPv6 ? "0" : "1", 0) < 0) {
+    if (virFileWriteStr(disable_ipv6, enableIPv6 ? "0" : "1", 0) < 0) {
         virReportSystemError(errno,
                              _("cannot write to %s to enable/disable IPv6 "
-                               "on bridge %s"), field, def->bridge);
+                               "on bridge %s"), disable_ipv6, 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,30 +2283,29 @@ networkSetIPv6Sysctls(virNetworkObjPtr obj)
     /* Prevent guests from hijacking the host network by sending out
      * their own router advertisements.
      */
-    field = g_strdup_printf(SYSCTL_PATH "/net/ipv6/conf/%s/accept_ra",
-                            def->bridge);
+    accept_ra = g_strdup_printf(SYSCTL_PATH "/net/ipv6/conf/%s/accept_ra",
+                                def->bridge);
 
-    if (virFileWriteStr(field, "0", 0) < 0) {
+    if (virFileWriteStr(accept_ra, "0", 0) < 0) {
         virReportSystemError(errno,
-                             _("cannot disable %s"), field);
+                             _("cannot disable %s"), accept_ra);
         goto cleanup;
     }
-    VIR_FREE(field);
 
     /* All interfaces used as a gateway (which is what this is, by
      * definition), must always have autoconf=0.
      */
-    field = g_strdup_printf(SYSCTL_PATH "/net/ipv6/conf/%s/autoconf", def->bridge);
+    autoconf = g_strdup_printf(SYSCTL_PATH "/net/ipv6/conf/%s/autoconf",
+                               def->bridge);
 
-    if (virFileWriteStr(field, "0", 0) < 0) {
+    if (virFileWriteStr(autoconf, "0", 0) < 0) {
         virReportSystemError(errno,
-                             _("cannot disable %s"), field);
+                             _("cannot disable %s"), autoconf);
         goto cleanup;
     }
 
     ret = 0;
  cleanup:
-    VIR_FREE(field);
     return ret;
 }
 
-- 
2.25.4

Re: [PATCH] network: refactor networkSetIPv6Sysctls() for proper g_autofree usage
Posted by Ján Tomko 3 years, 9 months ago
On a Friday in 2020, Laine Stump wrote:
>This function used the same char* three times for different purposes,
>freeing it after each use. Since we don't want to ever manually free
>an autofree'd pointer, modify it to use three separate char*, and make
>them all g_autofree.
>
>Signed-off-by: Laine Stump <laine@redhat.com>
>---
>This was suggested by Jan in
>
>  https://www.redhat.com/archives/libvir-list/2020-July/msg00805.html
>

Oops, I should have been more verboser.

Just a note for the mailing list that this was resolved by my:
commit 5c50d1dda5664d480e6370111c0218947706bd31
     network: split out networkSetIPv6Sysctl

which removed the repetitive use by moving it to a separate function.

Jano

>pushing this patch along with the patch 5 referenced there will permit
>pushing patch 06/15 of that series unmodified.
>
> src/network/bridge_driver.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)