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

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: Jim Meyering <meyering(a)redhat.com&gt; 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