[libvirt] [PATCH v2 10/32] storage: Use VIR_AUTOPTR(virString)

[Ján Tomko]

On Fri, Feb 08, 2019 at 01:37:04PM -0500, John Ferlan wrote:
>Let's make use of the auto __cleanup capabilities cleaning up any
>now unnecessary goto paths.
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>Reviewed-by: Erik Skultety <eskultet at redhat.com>
>---
> src/storage/storage_backend_iscsi_direct.c |  3 +-
> src/storage/storage_backend_sheepdog.c     |  6 +--
> src/storage/storage_backend_zfs.c          | 15 ++----
> src/util/virstoragefile.c                  | 58 ++++++++--------------
> 4 files changed, 28 insertions(+), 54 deletions(-)
>
>diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c
>index 42060dd758..cf48c29cde 100644
>--- a/src/storage/storage_backend_iscsi_direct.c
>+++ b/src/storage/storage_backend_iscsi_direct.c
>@@ -411,7 +411,7 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi,
>     struct iscsi_discovery_address *addr;
>     struct iscsi_discovery_address *tmp_addr;
>     size_t tmp_ntargets = 0;
>-    char **tmp_targets = NULL;
>+    VIR_AUTOPTR(virString) tmp_targets = NULL;
>
>     if (!(addr = iscsi_discovery_sync(iscsi))) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>@@ -439,7 +439,6 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi,
>     ret = 0;
>  cleanup:
>     iscsi_free_discovery_data(iscsi, addr);
>-    virStringListFreeCount(tmp_targets, tmp_ntargets);
>     return ret;
> }
>

virstring.h says:
VIR_DEFINE_AUTOPTR_FUNC(virString, virStringListFree);

Using virStringListFree instead of virStringListFreeCount might access
elements outside of the array, so this hunk should be dropped.

>diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
>index 1eb1ede59d..fc26c2f22e 100644
>--- a/src/util/virstoragefile.c
>+++ b/src/util/virstoragefile.c
>@@ -1594,11 +1594,10 @@ virStorageFileParseBackingStoreStr(const char *str,
>                                    char **target,
>                                    unsigned int *chainIndex)
> {
>-    char **strings = NULL;
>     size_t nstrings;
>     unsigned int idx = 0;
>     char *suffix;
>-    int ret = -1;
>+    VIR_AUTOPTR(virString) strings = NULL;
>
>     *chainIndex = 0;
>
>@@ -1608,19 +1607,15 @@ virStorageFileParseBackingStoreStr(const char *str,
>     if (nstrings == 2) {
>         if (virStrToLong_uip(strings[1], &suffix, 10, &idx) < 0 ||
>             STRNEQ(suffix, "]"))
>-            goto cleanup;
>+            return -1;
>     }
>
>     if (target &&
>         VIR_STRDUP(*target, strings[0]) < 0)
>-        goto cleanup;
>+        return -1;
>
>     *chainIndex = idx;
>-    ret = 0;
>-
>- cleanup:
>-    virStringListFreeCount(strings, nstrings);
>-    return ret;
>+    return 0;
> }

It's OK here, virStringSplitCount returns a NULL-terminated array of
strings.

>
>

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190211/11653900/attachment-0001.sig>