[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]