Here's proactive clean-up that is IMHO required. Otherwise, it's just too easy not to realize that "meta" must be cleared before each and every call to virStorageFileGetMetadata and virStorageFileGetMetadataFromFD. Besides, any change that adds just 2 lines and removes 11, in addition to making the code harder to abuse must be an improvement.

From 837732f1e307208b52721ffba83102e0edc361a7 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Tue, 18 May 2010 07:53:31 +0200 Subject: [PATCH] initialize "meta" in virStorageFileGetMetadata, not in each caller

Do not require each caller of virStorageFileGetMetadata and virStorageFileGetMetadataFromFD to first clear the storage of the "meta" buffer. Instead, initialize that storage in virStorageFileGetMetadataFromFD. * src/util/storage_file.c (virStorageFileGetMetadataFromFD): Clear "meta" here, not before each of the following callers. * src/qemu/qemu_driver.c (qemuSetupDiskCgroup): Don't clear "meta" here. (qemuTeardownDiskCgroup): Likewise. * src/qemu/qemu_security_dac.c (qemuSecurityDACSetSecurityImageLabel): Likewise. * src/security/security_selinux.c (SELinuxSetSecurityImageLabel): Likewise. * src/security/virt-aa-helper.c (get_files): Likewise. --- src/qemu/qemu_driver.c | 5 ----- src/qemu/qemu_security_dac.c | 2 -- src/security/security_selinux.c | 2 -- src/security/virt-aa-helper.c | 2 -- src/util/storage_file.c | 2 ++ 5 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e025987..0e70b1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2982,8 +2982,6 @@ static int qemuSetupDiskCgroup(virCgroupPtr cgroup, } } - memset(&meta, 0, sizeof(meta)); - rc = virStorageFileGetMetadata(path, &meta); if (path != disk->src) @@ -3030,8 +3028,6 @@ static int qemuTeardownDiskCgroup(virCgroupPtr cgroup, } } - memset(&meta, 0, sizeof(meta)); - rc = virStorageFileGetMetadata(path, &meta); if (path != disk->src) @@ -9316,7 +9312,6 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, } /* Probe for magic formats */ - memset(&meta, 0, sizeof(meta)); if (virStorageFileGetMetadataFromFD(path, fd, &meta) < 0) goto cleanup; diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index a816441..52150f7 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -115,8 +115,6 @@ qemuSecurityDACSetSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED, virStorageFileMetadata meta; int ret; - memset(&meta, 0, sizeof(meta)); - ret = virStorageFileGetMetadata(path, &meta); if (path != disk->src) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 669ef42..d90e17c 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -454,8 +454,6 @@ SELinuxSetSecurityImageLabel(virDomainObjPtr vm, virStorageFileMetadata meta; int ret; - memset(&meta, 0, sizeof(meta)); - ret = virStorageFileGetMetadata(path, &meta); if (path != disk->src) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 88cdc9d..c66f107 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -830,8 +830,6 @@ get_files(vahControl * ctl) do { virStorageFileMetadata meta; - memset(&meta, 0, sizeof(meta)); - ret = virStorageFileGetMetadata(path, &meta); if (path != ctl->def->disks[i]->src) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index a07bedc..b3ae905 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -275,6 +275,8 @@ virStorageFileGetMetadataFromFD(const char *path, unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ int len, i; + memset(meta, 0, sizeof (*meta)); + /* If all else fails, call it a raw file */ meta->format = VIR_STORAGE_FILE_RAW; -- 1.7.1.250.g7d1e8