[libvirt] [PATCH v1 02/32] util: iscsi: use VIR_AUTOPTR for aggregate types

Sukrit Bhatnagar posted 32 patches 7 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH v1 02/32] util: iscsi: use VIR_AUTOPTR for aggregate types
Posted by Sukrit Bhatnagar 7 years, 6 months ago
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.

Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
---
 src/util/viriscsi.c | 68 +++++++++++++++++------------------------------------
 1 file changed, 22 insertions(+), 46 deletions(-)

diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
index 48f4106..81404f8 100644
--- a/src/util/viriscsi.c
+++ b/src/util/viriscsi.c
@@ -91,7 +91,7 @@ virISCSIGetSession(const char *devpath,
     VIR_AUTOFREE(char *) error = NULL;
     int exitstatus = 0;
 
-    virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode",
+    VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode",
                                              "session", NULL);
     virCommandSetErrorBuffer(cmd, &error);
 
@@ -101,15 +101,13 @@ virISCSIGetSession(const char *devpath,
                            vars,
                            virISCSIExtractSession,
                            &cbdata, NULL, &exitstatus) < 0)
-        goto cleanup;
+        return NULL;
 
     if (cbdata.session == NULL && !probe)
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("cannot find iscsiadm session: %s"),
                        NULLSTR(error));
 
- cleanup:
-    virCommandFree(cmd);
     return cbdata.session;
 }
 
@@ -128,7 +126,7 @@ virStorageBackendIQNFound(const char *initiatoriqn,
     VIR_AUTOFREE(char *) outbuf = NULL;
     VIR_AUTOFREE(char *) iface = NULL;
     VIR_AUTOFREE(char *) iqn = NULL;
-    virCommandPtr cmd = virCommandNewArgList(ISCSIADM,
+    VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM,
                                              "--mode", "iface", NULL);
 
     *ifacename = NULL;
@@ -196,7 +194,6 @@ virStorageBackendIQNFound(const char *initiatoriqn,
     if (ret == IQN_MISSING)
         VIR_DEBUG("Could not find interface with IQN '%s'", iqn);
 
-    virCommandFree(cmd);
     return ret;
 
  error:
@@ -213,7 +210,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
 {
     int ret = -1, exitstatus = -1;
     VIR_AUTOFREE(char *) temp_ifacename = NULL;
-    virCommandPtr cmd = NULL;
+    VIR_AUTOPTR(virCommand) cmd = NULL;
 
     if (virAsprintf(&temp_ifacename,
                     "libvirt-iface-%08llx",
@@ -272,7 +269,6 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
     ret = 0;
 
  cleanup:
-    virCommandFree(cmd);
     if (ret != 0)
         VIR_FREE(*ifacename);
     return ret;
@@ -285,7 +281,6 @@ virISCSIConnection(const char *portal,
                    const char *target,
                    const char **extraargv)
 {
-    int ret = -1;
     const char *const baseargv[] = {
         ISCSIADM,
         "--mode", "node",
@@ -293,7 +288,7 @@ virISCSIConnection(const char *portal,
         "--targetname", target,
         NULL
     };
-    virCommandPtr cmd;
+    VIR_AUTOPTR(virCommand) cmd = NULL;
     VIR_AUTOFREE(char *) ifacename = NULL;
 
     cmd = virCommandNewArgs(baseargv);
@@ -306,7 +301,7 @@ virISCSIConnection(const char *portal,
             break;
         case IQN_MISSING:
             if (virStorageBackendCreateIfaceIQN(initiatoriqn, &ifacename) != 0)
-                goto cleanup;
+                return -1;
             /*
              * iscsiadm doesn't let you send commands to the Interface IQN,
              * unless you've first issued a 'sendtargets' command to the
@@ -317,25 +312,20 @@ virISCSIConnection(const char *portal,
              */
             if (virISCSIScanTargetsInternal(portal, ifacename,
                                             true, NULL, NULL) < 0)
-                goto cleanup;
+                return -1;
 
             break;
         case IQN_ERROR:
         default:
-            goto cleanup;
+            return -1;
         }
         virCommandAddArgList(cmd, "--interface", ifacename, NULL);
     }
 
     if (virCommandRun(cmd, NULL) < 0)
-        goto cleanup;
+        return -1;
 
-    ret = 0;
-
- cleanup:
-    virCommandFree(cmd);
-
-    return ret;
+    return 0;
 }
 
 
@@ -362,14 +352,12 @@ virISCSIConnectionLogout(const char *portal,
 int
 virISCSIRescanLUNs(const char *session)
 {
-    virCommandPtr cmd = virCommandNewArgList(ISCSIADM,
+    VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM,
                                              "--mode", "session",
                                              "-r", session,
                                              "-R",
                                              NULL);
-    int ret = virCommandRun(cmd, NULL);
-    virCommandFree(cmd);
-    return ret;
+    return virCommandRun(cmd, NULL);
 }
 
 
@@ -419,8 +407,7 @@ virISCSIScanTargetsInternal(const char *portal,
     int vars[] = { 2 };
     struct virISCSITargetList list;
     size_t i;
-    int ret = -1;
-    virCommandPtr cmd = virCommandNewArgList(ISCSIADM,
+    VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM,
                                              "--mode", "discovery",
                                              "--type", "sendtargets",
                                              "--portal", portal,
@@ -446,7 +433,7 @@ virISCSIScanTargetsInternal(const char *portal,
                            vars,
                            virISCSIGetTargets,
                            &list, NULL, NULL) < 0)
-        goto cleanup;
+        return -1;
 
     if (ntargetsret && targetsret) {
         *ntargetsret = list.ntargets;
@@ -457,10 +444,7 @@ virISCSIScanTargetsInternal(const char *portal,
         VIR_FREE(list.targets);
     }
 
-    ret = 0;
- cleanup:
-    virCommandFree(cmd);
-    return ret;
+    return 0;
 }
 
 
@@ -538,9 +522,8 @@ int
 virISCSINodeNew(const char *portal,
                 const char *target)
 {
-    virCommandPtr cmd = NULL;
+    VIR_AUTOPTR(virCommand) cmd = NULL;
     int status;
-    int ret = -1;
 
     cmd = virCommandNewArgList(ISCSIADM,
                                "--mode", "node",
@@ -553,20 +536,17 @@ virISCSINodeNew(const char *portal,
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed new node mode for target '%s'"),
                        target);
-        goto cleanup;
+        return -1;
     }
 
     if (status != 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("%s failed new mode for target '%s' with status '%d'"),
                        ISCSIADM, target, status);
-        goto cleanup;
+        return -1;
     }
 
-    ret = 0;
- cleanup:
-    virCommandFree(cmd);
-    return ret;
+    return 0;
 }
 
 
@@ -576,9 +556,8 @@ virISCSINodeUpdate(const char *portal,
                    const char *name,
                    const char *value)
 {
-    virCommandPtr cmd = NULL;
+    VIR_AUTOPTR(virCommand) cmd = NULL;
     int status;
-    int ret = -1;
 
     cmd = virCommandNewArgList(ISCSIADM,
                                "--mode", "node",
@@ -594,11 +573,8 @@ virISCSINodeUpdate(const char *portal,
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to update '%s' of node mode for target '%s'"),
                        name, target);
-        goto cleanup;
+        return -1;
     }
 
-    ret = 0;
- cleanup:
-    virCommandFree(cmd);
-    return ret;
+    return 0;
 }
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 02/32] util: iscsi: use VIR_AUTOPTR for aggregate types
Posted by Erik Skultety 7 years, 6 months ago
On Sat, Jul 28, 2018 at 11:31:17PM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> ---
>  src/util/viriscsi.c | 68 +++++++++++++++++------------------------------------
>  1 file changed, 22 insertions(+), 46 deletions(-)
>
> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
> index 48f4106..81404f8 100644
> --- a/src/util/viriscsi.c
> +++ b/src/util/viriscsi.c
> @@ -91,7 +91,7 @@ virISCSIGetSession(const char *devpath,
>      VIR_AUTOFREE(char *) error = NULL;
>      int exitstatus = 0;
>
> -    virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode",
> +    VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode",
>                                               "session", NULL);

There are a few code misalignment issues like ^this. I'll fix them locally.

...

> @@ -576,9 +556,8 @@ virISCSINodeUpdate(const char *portal,
>                     const char *name,
>                     const char *value)
>  {
> -    virCommandPtr cmd = NULL;
> +    VIR_AUTOPTR(virCommand) cmd = NULL;

One tiny nitpick I realized only now is that we should probably start being
consistent in what order we declare the autoclean variables, IOW I think we
should group all the autoclean variables together so visually you can
immediately tell which variables are handled automatically and which aren't. It
was all fine across this file except for ^this single occurrence, I'll fix
that.

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list