[PATCH] lxcFreezeContainer: avoid test-after-deref of never-NULL pointer

[Jim Meyering <jim@xxxxxxxxxxxx>]

This addresses another coverity-spotted "flaw".
However, since "cgroup" is never NULL after that initial "if" stmt,
the only penalty is that the useless cleanup test would make a reviewer
try to figure out how cgroup could be NULL there.

>From d89098801d4e5011e07994cf0391ace2363d8971 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Mon, 17 May 2010 19:18:12 +0200
Subject: [PATCH] lxcFreezeContainer: avoid test-after-deref of never-NULL pointer

* src/lxc/lxc_driver.c (lxcFreezeContainer): Remove test-after-deref.
Correct indentation in expression.
---
 src/lxc/lxc_driver.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index fc0df37..8c3bbd3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2276,69 +2276,71 @@ static int lxcDomainSetAutostart(virDomainPtr dom,
         }
     } else {
         if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) {
             virReportSystemError(errno,
                                  _("Failed to delete symlink '%s'"),
                                  autostartLink);
             goto cleanup;
         }
     }

     vm->autostart = autostart;
     ret = 0;

 cleanup:
     VIR_FREE(configFile);
     VIR_FREE(autostartLink);
     if (vm)
         virDomainObjUnlock(vm);
     lxcDriverUnlock(driver);
     return ret;
 }

 static int lxcFreezeContainer(lxc_driver_t *driver, virDomainObjPtr vm)
 {
     int timeout = 1000; /* In milliseconds */
     int check_interval = 1; /* In milliseconds */
     int exp = 10;
     int waited_time = 0;
     int ret = -1;
     char *state = NULL;
     virCgroupPtr cgroup = NULL;

     if (!(driver->cgroup &&
-        virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0))
+          virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0))
         return -1;

+    /* From here on, we know that cgroup != NULL.  */
+
     while (waited_time < timeout) {
         int r;
         /*
          * Writing "FROZEN" to the "freezer.state" freezes the group,
          * i.e., the container, temporarily transiting "FREEZING" state.
          * Once the freezing is completed, the state of the group transits
          * to "FROZEN".
          * (see linux-2.6/Documentation/cgroups/freezer-subsystem.txt)
          */
         r = virCgroupSetFreezerState(cgroup, "FROZEN");

         /*
          * Returning EBUSY explicitly indicates that the group is
          * being freezed but incomplete and other errors are true
          * errors.
          */
         if (r < 0 && r != -EBUSY) {
             VIR_DEBUG("Writing freezer.state failed with errno: %d", r);
             goto error;
         }
         if (r == -EBUSY)
             VIR_DEBUG0("Writing freezer.state gets EBUSY");

         /*
          * Unfortunately, returning 0 (success) is likely to happen
          * even when the freezing has not been completed. Sometimes
          * the state of the group remains "FREEZING" like when
          * returning -EBUSY and even worse may never transit to
          * "FROZEN" even if writing "FROZEN" again.
          *
          * So we don't trust the return value anyway and always
          * decide that the freezing has been complete only with
          * the state actually transit to "FROZEN".
@@ -2351,68 +2353,67 @@ static int lxcFreezeContainer(lxc_driver_t *driver, virDomainObjPtr vm)
             VIR_DEBUG("Reading freezer.state failed with errno: %d", r);
             goto error;
         }
         VIR_DEBUG("Read freezer.state: %s", state);

         if (STREQ(state, "FROZEN")) {
             ret = 0;
             goto cleanup;
         }

         waited_time += check_interval;
         /*
          * Increasing check_interval exponentially starting with
          * small initial value treats nicely two cases; One is
          * a container is under no load and waiting for long period
          * makes no sense. The other is under heavy load. The container
          * may stay longer time in FREEZING or never transit to FROZEN.
          * In that case, eager polling will just waste CPU time.
          */
         check_interval *= exp;
         VIR_FREE(state);
     }
     VIR_DEBUG0("lxcFreezeContainer timeout");
 error:
     /*
      * If timeout or an error on reading the state occurs,
      * activate the group again and return an error.
      * This is likely to fall the group back again gracefully.
      */
     virCgroupSetFreezerState(cgroup, "THAWED");
     ret = -1;

 cleanup:
-    if (cgroup)
-        virCgroupFree(&cgroup);
+    virCgroupFree(&cgroup);
     VIR_FREE(state);
     return ret;
 }

 static int lxcDomainSuspend(virDomainPtr dom)
 {
     lxc_driver_t *driver = dom->conn->privateData;
     virDomainObjPtr vm;
     virDomainEventPtr event = NULL;
     int ret = -1;

     lxcDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);

     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
         virUUIDFormat(dom->uuid, uuidstr);
         lxcError(VIR_ERR_NO_DOMAIN,
                  _("No domain with matching uuid '%s'"), uuidstr);
         goto cleanup;
     }

     if (!virDomainObjIsActive(vm)) {
         lxcError(VIR_ERR_OPERATION_INVALID,
                  "%s", _("Domain is not running"));
         goto cleanup;
     }

     if (vm->state != VIR_DOMAIN_PAUSED) {
         if (lxcFreezeContainer(driver, vm) < 0) {
             lxcError(VIR_ERR_OPERATION_FAILED,
                      "%s", _("Suspend operation failed"));
             goto cleanup;
--
1.7.1.250.g7d1e8

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list