On 05/03/2011 05:49 PM, Eric Blake wrote:
Clang detected a dead store to rc. It turns out that in fixing
this,
I also found a FILE* leak.
* src/util/cgroup.c (virCgroupKillInternal): Abort rather than
resuming loop on fscanf failure, and cleanup file on error.
This definitely fixes the FILE* leak, but do we really want to abort the
loop (stop looking for more pidfiles) when fscanf fails on one pidfile?
(dunno, just asking)
---
As a bonus, this also fixes a decl-after-statement for fp.
src/util/cgroup.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index afe8731..62b371d 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -1351,7 +1351,9 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr
int killedAny = 0;
char *keypath = NULL;
bool done = false;
- VIR_DEBUG("group=%p path=%s signum=%d pids=%p", group, group->path,
signum, pids);
+ FILE *fp = NULL;
+ VIR_DEBUG("group=%p path=%s signum=%d pids=%p",
+ group, group->path, signum, pids);
rc = virCgroupPathOfController(group, -1, "tasks",&keypath);
if (rc != 0) {
@@ -1364,7 +1366,6 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr
*/
while (!done) {
done = true;
- FILE *fp;
if (!(fp = fopen(keypath, "r"))) {
rc = -errno;
VIR_DEBUG("Failed to read %s: %m\n", keypath);
@@ -1376,7 +1377,8 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr
if (feof(fp))
break;
rc = -errno;
- break;
+ VIR_DEBUG("Failed to read %s: %m\n", keypath);
+ goto cleanup;
}
if (virHashLookup(pids, (void*)pid))
continue;
@@ -1403,6 +1405,7 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr
cleanup:
VIR_FREE(keypath);
+ VIR_FORCE_FCLOSE(fp);
return rc;
}