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