virStorageFileChainLookup: Return virStorageSourcePtr

Returning both virStorageSourcePtr and its path member does not make a
lot of sense.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

src/qemu/qemu_driver.c

@@ -15329,9 +15329,8 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
 
     if (!top) {
         topSource = &disk->src;
-    } else if (!(virStorageFileChainLookup(&disk->src,
+    } else if (!(topSource = virStorageFileChainLookup(disk->src.backingStore,
-                                           top, &topSource,
+                                                       top, &top_parent))) {
-                                           &top_parent))) {
         goto endjob;
     }
     if (!topSource->backingStore) {
@@ -15343,7 +15342,7 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
 
     if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
         baseSource = topSource->backingStore;
-    else if (!(virStorageFileChainLookup(topSource, base, &baseSource, NULL)))
+    else if (!(baseSource = virStorageFileChainLookup(topSource, base, NULL)))
         goto endjob;
 
     if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&

src/util/virstoragefile.c

@@ -1516,9 +1516,9 @@ int virStorageFileGetSCSIKey(const char *path,
  * Since the results point within CHAIN, they must not be
  * independently freed.  Reports an error and returns NULL if NAME is
  * not found.  */
-const char *
+virStorageSourcePtr
 virStorageFileChainLookup(virStorageSourcePtr chain,
-                          const char *name, virStorageSourcePtr *meta,
+                          const char *name,
                           const char **parent)
 {
     const char *start = chain->path;
@@ -1551,24 +1551,22 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
         parentDir = chain->relDir;
         chain = chain->backingStore;
     }
+
     if (!chain)
         goto error;
-    if (meta)
+    return chain;
-        *meta = chain;
-    return chain->path;
 
  error:
-    if (name)
+    if (name) {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("could not find image '%s' in chain for '%s'"),
                        name, start);
-    else
+    } else {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("could not find base image in chain for '%s'"),
                        start);
+    }
     *parent = NULL;
-    if (meta)
-        *meta = NULL;
     return NULL;
 }
 

src/util/virstoragefile.h

@@ -281,10 +281,9 @@ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path,
 int virStorageFileChainGetBroken(virStorageSourcePtr chain,
                                  char **broken_file);
 
-const char *virStorageFileChainLookup(virStorageSourcePtr chain,
+virStorageSourcePtr virStorageFileChainLookup(virStorageSourcePtr chain,
-                                      const char *name,
+                                              const char *name,
-                                      virStorageSourcePtr *meta,
+                                              const char **parent)
-                                      const char **parent)
     ATTRIBUTE_NONNULL(1);
 
 int virStorageFileResize(const char *path,

tests/virstoragetest.c

@@ -424,16 +424,11 @@ testStorageLookup(const void *args)
 {
     const struct testLookupData *data = args;
     int ret = 0;
-    const char *actualResult;
+    virStorageSourcePtr result;
-    virStorageSourcePtr actualMeta;
     const char *actualParent;
 
-    /* This function is documented as giving results within chain, but
+     /* Test twice to ensure optional parameter doesn't cause NULL deref. */
-     * as the same string may be duplicated into more than one field,
+    result = virStorageFileChainLookup(data->chain, data->name, NULL);
-     * we rely on STREQ rather than pointer equality.  Test twice to
-     * ensure optional parameters don't cause NULL deref.  */
-    actualResult = virStorageFileChainLookup(data->chain, data->name,
-                                             NULL, NULL);
 
     if (!data->expResult) {
         if (!virGetLastError()) {
@@ -448,24 +443,36 @@ testStorageLookup(const void *args)
         }
     }
 
-    if (STRNEQ_NULLABLE(data->expResult, actualResult)) {
+    if (!result) {
+        if (data->expResult) {
+            fprintf(stderr, "result 1: expected %s, got NULL\n",
+                    data->expResult);
+            ret = -1;
+        }
+    } else if (STRNEQ_NULLABLE(data->expResult, result->path)) {
         fprintf(stderr, "result 1: expected %s, got %s\n",
-                NULLSTR(data->expResult), NULLSTR(actualResult));
+                NULLSTR(data->expResult), NULLSTR(result->path));
         ret = -1;
     }
 
-    actualResult = virStorageFileChainLookup(data->chain, data->name,
+    result = virStorageFileChainLookup(data->chain, data->name, &actualParent);
-                                             &actualMeta, &actualParent);
     if (!data->expResult)
         virResetLastError();
-    if (STRNEQ_NULLABLE(data->expResult, actualResult)) {
+
+    if (!result) {
+        if (data->expResult) {
+            fprintf(stderr, "result 2: expected %s, got NULL\n",
+                    data->expResult);
+            ret = -1;
+        }
+    } else if (STRNEQ_NULLABLE(data->expResult, result->path)) {
         fprintf(stderr, "result 2: expected %s, got %s\n",
-                NULLSTR(data->expResult), NULLSTR(actualResult));
+                NULLSTR(data->expResult), NULLSTR(result->path));
         ret = -1;
     }
-    if (data->expMeta != actualMeta) {
+    if (data->expMeta != result) {
         fprintf(stderr, "meta: expected %p, got %p\n",
-                data->expMeta, actualMeta);
+                data->expMeta, result);
         ret = -1;
     }
     if (STRNEQ_NULLABLE(data->expParent, actualParent)) {