2011/7/14 Eric Blake <eblake@redhat.com>:
On 07/14/2011 11:42 AM, Matthias Bolte wrote:
2011/7/12 Eric Blake <eblake@redhat.com>:
By requesting the pid in virCommandRunAsync, fdstream was claiming that it would manually wait for the process. But on the failure path, the child process was being leaked.
This difference in behavior between virCommandRunAsync(..., NULL) and virCommandRunAsync(..., &pid) is not documented on virCommandRunAsync itself but only on virCommandFree.
That was the case in libvirt.git right now, but I thought I addressed that properly in patch 1/3.
Oh, true. So that's okay now.
Also virCommandFree refers to waitpid instead of you're newly added virPidWait.
Oh, good point. I'll tweak patch 1 to fix that before I push.
Finally I noticed that the comments in command.h and command.c are out of sync (at least for virCommandFree). Why are you documenting in two places making it prone to get out of sync?
Which file is better, the .c or the .h? I'm fine with dropping docs from one of the two locations, if we have a preference on which file is more likely to be referenced.
.c: PRO - the documentation is next to the implementation, and hopefully easier to keep the two in sync PRO - matches how we do things in libvirt.h vs. libvirt.c CON - much larger file to search through when you are looking up the reference .h: PRO - smaller file CON - further away from implementation, could go stale
so I'm leaning towards .c only.
I prefer .c, but don't just drop them from .h. merge them instead :) -- Matthias Bolte http://photron.blogspot.com