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/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list