virDevMapperGetTargets: Use a linked list as return type

Of the two callers one simply iterates over the returned paths and the
second one appends the returned paths to another linked list. Simplify
all of this by directly returning a linked list.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 6d4a82b3cdfe9fd715b365631d0141c441783103..471cbc3b8f719129ceb9639f0808fe18f6149164 100644 (file)
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -38,6 +38,7 @@
 #include "virnuma.h"
 #include "virdevmapper.h"
 #include "virutil.h"
+#include "virglibutil.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -60,8 +61,8 @@ qemuSetupImagePathCgroup(virDomainObj *vm,
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     int perms = VIR_CGROUP_DEVICE_READ;
-    g_auto(GStrv) targetPaths = NULL;
-    size_t i;
+    g_autoptr(virGSListString) targetPaths = NULL;
+    GSList *n;
     int rv;
 
     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
@@ -94,10 +95,10 @@ qemuSetupImagePathCgroup(virDomainObj *vm,
         return -1;
     }
 
-    for (i = 0; targetPaths && targetPaths[i]; i++) {
-        rv = virCgroupAllowDevicePath(priv->cgroup, targetPaths[i], perms, false);
+    for (n = targetPaths; n; n = n->next) {
+        rv = virCgroupAllowDevicePath(priv->cgroup, n->data, perms, false);
 
-        virDomainAuditCgroupPath(vm, priv->cgroup, "allow", targetPaths[i],
+        virDomainAuditCgroupPath(vm, priv->cgroup, "allow", n->data,
                                  virCgroupGetDevicePermsString(perms),
                                  rv);
         if (rv < 0)

diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 728d77fc6149d996ca9df2d73e2f5cdecc40b61c..f1aaca86b135ec5eaa5f8794c55e374c842d295c 100644 (file)
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -251,8 +251,7 @@ qemuDomainSetupDisk(virStorageSource *src,
             if (!(tmpPath = virPCIDeviceAddressGetIOMMUGroupDev(&next->nvme->pciAddr)))
                 return -1;
         } else {
-            g_auto(GStrv) targetPaths = NULL;
-            GStrv tmp;
+            GSList *targetPaths;
 
             if (virStorageSourceIsEmpty(next) ||
                 !virStorageSourceIsLocalStorage(next)) {
@@ -270,10 +269,8 @@ qemuDomainSetupDisk(virStorageSource *src,
                 return -1;
             }
 
-            if (targetPaths) {
-                for (tmp = targetPaths; *tmp; tmp++)
-                    *paths = g_slist_prepend(*paths, g_steal_pointer(tmp));
-            }
+            if (targetPaths)
+                *paths = g_slist_concat(g_slist_reverse(targetPaths), *paths);
         }
 
         *paths = g_slist_prepend(*paths, g_steal_pointer(&tmpPath));

diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 301c8f3ba7bfbb09453973515566200b358b5a85..e42324fd19468339030cdd49ea8cb001c9ad9713 100644 (file)
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -36,6 +36,7 @@
 # include "virstring.h"
 # include "virfile.h"
 # include "virlog.h"
+# include "virglibutil.h"
 
 # define VIR_FROM_THIS VIR_FROM_STORAGE
 
@@ -217,18 +218,16 @@ virDMSanitizepath(const char *path)
 static int
 virDevMapperGetTargetsImpl(int controlFD,
                            const char *path,
-                           char ***devPaths_ret,
+                           GSList **devPaths,
                            unsigned int ttl)
 {
     g_autofree char *sanitizedPath = NULL;
     g_autofree char *buf = NULL;
     struct dm_ioctl dm;
     struct dm_target_deps *deps = NULL;
-    g_auto(GStrv) devPaths = NULL;
     size_t i;
 
     memset(&dm, 0, sizeof(dm));
-    *devPaths_ret = NULL;
 
     if (ttl == 0) {
         errno = ELOOP;
@@ -258,24 +257,17 @@ virDevMapperGetTargetsImpl(int controlFD,
         return -1;
     }
 
-    devPaths = g_new0(char *, deps->count + 1);
     for (i = 0; i < deps->count; i++) {
-        devPaths[i] = g_strdup_printf("/dev/block/%u:%u",
-                                      major(deps->dev[i]),
-                                      minor(deps->dev[i]));
-    }
-
-    for (i = 0; i < deps->count; i++) {
-        g_auto(GStrv) tmpPaths = NULL;
+        char *curpath = g_strdup_printf("/dev/block/%u:%u",
+                                        major(deps->dev[i]),
+                                        minor(deps->dev[i]));
 
-        if (virDevMapperGetTargetsImpl(controlFD, devPaths[i], &tmpPaths, ttl - 1) < 0)
-            return -1;
+        *devPaths = g_slist_prepend(*devPaths, curpath);
 
-        if (virStringListMerge(&devPaths, &tmpPaths) < 0)
+        if (virDevMapperGetTargetsImpl(controlFD, curpath, devPaths, ttl - 1) < 0)
             return -1;
     }
 
-    *devPaths_ret = g_steal_pointer(&devPaths);
     return 0;
 }
 
@@ -283,11 +275,10 @@ virDevMapperGetTargetsImpl(int controlFD,
 /**
  * virDevMapperGetTargets:
  * @path: devmapper target
- * @devPaths: returned string list of devices
+ * @devPaths: filled in by a GSList containing the paths
  *
  * For given @path figure out its targets, and store them in
- * @devPaths array. Note, @devPaths is a string list so it's NULL
- * terminated.
+ * @devPaths.
  *
  * If @path is not a devmapper device, @devPaths is set to NULL and
  * success is returned.
@@ -301,10 +292,11 @@ virDevMapperGetTargetsImpl(int controlFD,
  */
 int
 virDevMapperGetTargets(const char *path,
-                       char ***devPaths)
+                       GSList **devPaths)
 {
     VIR_AUTOCLOSE controlFD = -1;
     const unsigned int ttl = 32;
+    g_autoptr(virGSListString) paths = NULL;
 
     /* Arbitrary limit on recursion level. A devmapper target can
      * consist of devices or yet another targets. If that's the
@@ -321,7 +313,11 @@ virDevMapperGetTargets(const char *path,
         return -1;
     }
 
-    return virDevMapperGetTargetsImpl(controlFD, path, devPaths, ttl);
+    if (virDevMapperGetTargetsImpl(controlFD, path, &paths, ttl) < 0)
+        return -1;
+
+    *devPaths = g_slist_reverse(g_steal_pointer(&paths));
+    return 0;
 }
 
 
@@ -346,7 +342,7 @@ virIsDevMapperDevice(const char *dev_name)
 
 int
 virDevMapperGetTargets(const char *path G_GNUC_UNUSED,
-                       char ***devPaths G_GNUC_UNUSED)
+                       GSlist **devPaths G_GNUC_UNUSED)
 {
     errno = ENOSYS;
     return -1;

diff --git a/src/util/virdevmapper.h b/src/util/virdevmapper.h
index 834900692eda42dc34e0d28ddadb4fe81b4c6103..4d8d3ccdb8f3c6cc6d2a0a18f39946ba9034efb8 100644 (file)
--- a/src/util/virdevmapper.h
+++ b/src/util/virdevmapper.h
@@ -24,7 +24,7 @@
 
 int
 virDevMapperGetTargets(const char *path,
-                       char ***devPaths) G_GNUC_NO_INLINE;
+                       GSList **devPaths) G_GNUC_NO_INLINE;
 
 bool
 virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1);

diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index d1fc50c5f0e60795c1d6d044af8c9ab1bd8a49bd..3b5f858044627554b0ce8703e572f38f5c07be3e 100644 (file)
--- a/tests/qemuhotplugmock.c
+++ b/tests/qemuhotplugmock.c
@@ -54,16 +54,14 @@ qemuDomainGetUnplugTimeout(virDomainObj *vm)
 
 int
 virDevMapperGetTargets(const char *path,
-                       char ***devPaths)
+                       GSList **devPaths)
 {
     *devPaths = NULL;
 
     if (STREQ(path, "/dev/mapper/virt")) {
-        *devPaths = g_new0(char *, 4);
-        (*devPaths)[0] = g_strdup("/dev/block/8:0");  /* /dev/sda */
-        (*devPaths)[1] = g_strdup("/dev/block/8:16"); /* /dev/sdb */
-        (*devPaths)[2] = g_strdup("/dev/block/8:32"); /* /dev/sdc */
-        (*devPaths)[3] = NULL;
+        *devPaths = g_slist_prepend(*devPaths, g_strdup("/dev/block/8:32")); /* /dev/sdc */
+        *devPaths = g_slist_prepend(*devPaths, g_strdup("/dev/block/8:16")); /* /dev/sdb */
+        *devPaths = g_slist_prepend(*devPaths, g_strdup("/dev/block/8:0")); /* /dev/sda */
     }
 
     return 0;