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