[libvirt] [PATCH] storage: kill dead stores

[Eric Blake]

Found by clang.  Clang complained that virStorageBackendProbeTarget
could dereference NULL if backingStoreFormat was NULL, but since all
callers passed a valid pointer, I added attributes instead of null
checks.

* src/storage/storage_backend.c
(virStorageBackendQEMUImgBackingFormat): Kill dead store.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Likewise.  Skip null checks, by adding attributes.
---

Thankfully, the null dereference scenario noted by clang was never
triggered in the code, which is good since it was introduced as
part of fixing a CVE.

 src/storage/storage_backend.c    |    3 +--
 src/storage/storage_backend_fs.c |   34 +++++++++++++++-------------------
 2 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index d989743..1fe7ba6 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -581,7 +581,6 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
     int newstdout = -1;
     char *help = NULL;
     enum { MAX_HELP_OUTPUT_SIZE = 1024*8 };
-    int len;
     char *start;
     char *end;
     char *tmp;
@@ -591,7 +590,7 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
                 &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0)
         goto cleanup;

-    if ((len = virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help)) < 0) {
+    if (virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help) < 0) {
         virReportSystemError(errno,
                              _("Unable to read '%s -h' output"),
                              qemuimg);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 0761bd8..b6b8fdd 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -48,7 +48,7 @@

 #define VIR_FROM_THIS VIR_FROM_STORAGE

-static int
+static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
 virStorageBackendProbeTarget(virStorageVolTargetPtr target,
                              char **backingStore,
                              int *backingStoreFormat,
@@ -59,10 +59,8 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
     int fd, ret;
     virStorageFileMetadata meta;

-    if (backingStore)
-        *backingStore = NULL;
-    if (backingStoreFormat)
-        *backingStoreFormat = VIR_STORAGE_FILE_AUTO;
+    *backingStore = NULL;
+    *backingStoreFormat = VIR_STORAGE_FILE_AUTO;
     if (encryption)
         *encryption = NULL;

@@ -75,7 +73,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
                                                       allocation,
                                                       capacity)) < 0) {
         close(fd);
-        return -1;
+        return ret;
     }

     memset(&meta, 0, sizeof(meta));
@@ -95,20 +93,19 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
     close(fd);

     if (meta.backingStore) {
-        if (backingStore) {
-            *backingStore = meta.backingStore;
-            meta.backingStore = NULL;
-            if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
-                if ((*backingStoreFormat = virStorageFileProbeFormat(*backingStore)) < 0) {
-                    close(fd);
-                    goto cleanup;
-                }
-            } else {
-                *backingStoreFormat = meta.backingStoreFormat;
+        *backingStore = meta.backingStore;
+        meta.backingStore = NULL;
+        if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
+            if ((*backingStoreFormat
+                 = virStorageFileProbeFormat(*backingStore)) < 0) {
+                close(fd);
+                goto cleanup;
             }
         } else {
-            VIR_FREE(meta.backingStore);
+            *backingStoreFormat = meta.backingStoreFormat;
         }
+    } else {
+        VIR_FREE(meta.backingStore);
     }

     if (capacity && meta.capacity)
@@ -139,8 +136,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
     return 0;

 cleanup:
-    if (backingStore)
-        VIR_FREE(*backingStore);
+    VIR_FREE(*backingStore);
     return -1;
 }

-- 
1.7.2