Re: [libvirt] [PATCH] cgroup: avoid leaking a file

[Daniel Veillard <veillard redhat com>]

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/