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

[Eric Blake]

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.

> I don't know if that's important or not, but it is a change in behavior.

I've updated the commit message to call attention to the subtle (and
unlikely) change in behavior.

> 
>> ACK, but let's have a second patch for after 0.9.1
>> if we think this need improvement.

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.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110504/d2dacc63/attachment-0001.sig>