Re: [libvirt] [PATCH] (qemuTeardownDiskCgroup): avoid dead code
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(a)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