On Wed, May 04, 2011 at 12:30:07AM -0400, Laine Stump wrote:
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) [...]
@@ -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;
Seems to me we were doing a break anyway, so it should not change behaviour there, ACK, but let's have a second patch for after 0.9.1 if we think this need improvement. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/