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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list