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(a)redhat.com&gt; --- src/qemu/qemu_cgroup.c | 11 ++++++----- src/qemu/qemu_namespace.c | 9 +++------ src/util/virdevmapper.c | 38 +++++++++++++++++--------------------- src/util/virdevmapper.h | 2 +- tests/qemuhotplugmock.c | 10 ++++------ 5 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 6d4a82b3cd..471cbc3b8f 100644 --- 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 728d77fc61..f1aaca86b1 100644 --- 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 301c8f3ba7..e42324fd19 100644 --- 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 834900692e..4d8d3ccdb8 100644 --- 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 d1fc50c5f0..3b5f858044 100644 --- 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; -- 2.31.1

Use g_autofree for the JSON values to remove cleanup label and ret variable. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor_json.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8d3c4031a6..37e9c05d27 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1710,10 +1710,9 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon, bool *running, virDomainPausedReason *reason) { - int ret = -1; const char *status; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; virJSONValue *data; if (reason) @@ -1723,17 +1722,17 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon, return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + return -1; if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) - goto cleanup; + return -1; data = virJSONValueObjectGetObject(reply, "return"); if (virJSONValueObjectGetBoolean(data, "running", running) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-status reply was missing running state")); - goto cleanup; + return -1; } if ((status = virJSONValueObjectGetString(data, "status"))) { @@ -1743,12 +1742,7 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon, VIR_DEBUG("query-status reply was missing status details"); } - ret = 0; - - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return 0; } -- 2.31.1