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>

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)

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));

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",
+        char *curpath = 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;
+        *devPaths = g_slist_prepend(*devPaths, curpath);
 
-        if (virDevMapperGetTargetsImpl(controlFD, devPaths[i], &tmpPaths, ttl - 1) < 0)
-            return -1;
-
-        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;

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);

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;