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

[Laine Stump <laine laine org>]

On 05/04/2011 10:40 AM, Eric Blake wrote:
> On 05/04/2011 06:23 AM, Laine Stump wrote:
> > > > 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)
> Personally, I think that since pidfiles are always generated by the
> kernel, they should always have sane contents, so failure to read them
> means something is really wrong.  The fact that the overall function
> returns -1, right away, rather than trying to read other pid files (if
> any), is in some regards a feature (if we're having problems reading one
> pid file, then what else is going wrong?).  Furthermore, there were
> other places in the loop that aborted on the first failure, so this
> isn't the first case of aborting the outer loop if the inner loop fails.

Fair enough.

> I've pushed the patch unchanged for 0.9.1; right now, I don't think we
> need any post-0.9.1 improvement, but I'm still open to anyone else that
> can argue a case for trying to read remaining pid files even if we
> failed to read one.

Nah, not from me. I don't have an opinion on best practice in this case, 
just wanted to make sure the change was recognized and considered by 
anyone who might.