Eric Blake wrote:
On 05/17/2010 02:52 PM, Jim Meyering wrote:
It's a good thing the latter while loop condition could never be true -- otherwise it'd be an infloop.
static int qemuTeardownDiskCgroup(virCgroupPtr cgroup, virDomainObjPtr vm, virDomainDiskDefPtr disk) { char *path = disk->src; int ret = -1;
- while (path != NULL) { + do { virStorageFileMetadata meta; int rc;
VIR_DEBUG("Process path %s for disk", path); ... path = meta.backingStore; } while (path != NULL);
Are we sure disk->src is guaranteed to be non-NULL on entry, or would have been better to rewrite this as while{}/*nothing*/ instead of do{}while?
You're right. disk->src may be NULL, as attested by surrounding code that tests for it via NULLSTR upon failure of these functions. Thanks! Here's a better patch:
From 9fccf57a1c786612048964d7c5a54f5fc5e85d56 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 17 May 2010 22:50:21 +0200 Subject: [PATCH] (qemu*DiskCgroup): avoid dead code
* src/qemu/qemu_driver.c (qemuTeardownDiskCgroup): Remove bogus empty-body while-loop. (qemuSetupDiskCgroup): Likewise. --- src/qemu/qemu_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 16a9646..c29392d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2992,11 +2992,11 @@ static int qemuSetupDiskCgroup(virCgroupPtr cgroup, if (rc < 0) goto cleanup; path = meta.backingStore; - } while (path != NULL); + } ret = 0; cleanup: return ret; @@ -3040,11 +3040,11 @@ static int qemuTeardownDiskCgroup(virCgroupPtr cgroup, if (rc < 0) goto cleanup; path = meta.backingStore; - } while (path != NULL); + } ret = 0; cleanup: return ret; -- 1.7.1.250.g7d1e8