]> xenbits.xensource.com Git - libvirt.git/commitdiff
cgroup: avoid leaking a file
authorEric Blake <eblake@redhat.com>
Tue, 3 May 2011 21:46:06 +0000 (15:46 -0600)
committerEric Blake <eblake@redhat.com>
Wed, 4 May 2011 14:38:27 +0000 (08:38 -0600)
Clang detected a dead store to rc.  It turns out that in fixing this,
I also found a FILE* leak.

This is a subtle change in behavior, although unlikely to hit.  The
pidfile is a kernel file, so we've probably got more serious problems
under foot if we fail to parse one.  However, the previous behavior
was that even if one pid file failed to parse, we tried others,
whereas now we give up on the first failure.  Either way, though,
the function returns -1, so the caller will know that something is
going wrong, and that not all pids were necessarily reaped.  Besides,
there were other instances already in the code where failure in the
inner loop aborted the outer loop.

* src/util/cgroup.c (virCgroupKillInternal): Abort rather than
resuming loop on fscanf failure, and cleanup file on error.

src/util/cgroup.c

index afe873118a3a79ef2259f214afc07e75bf2382cb..62b371d7c48e2adc2a913af0715cb8d1920cb0a1 100644 (file)
@@ -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;
 }