[libvirt] [PATCHv2] waitpid: improve safety

[Eric Blake]

On 10/24/2011 01:58 AM, Daniel P. Berrange wrote:
> On Fri, Oct 21, 2011 at 02:30:28PM -0600, Eric Blake wrote:
>> Based on a report by Coverity.  waitpid() can leak resources if it
>> fails with EINTR, so it should never be used without checking return
>> status.  But we already have a helper function that does that, so
>> use it in more places.
>>

>> +++ b/src/lxc/lxc_controller.c
>> @@ -59,6 +59,7 @@
>>   #include "util.h"
>>   #include "virfile.h"
>>   #include "virpidfile.h"
>> +#include "command.h"
>>
>>   #define VIR_FROM_THIS VIR_FROM_LXC
>>
>> @@ -515,7 +516,8 @@ ignorable_epoll_accept_errno(int errnum)
>>   static bool
>>   lxcPidGone(pid_t container)
>>   {
>> -    waitpid(container, NULL, WNOHANG);
>> +    if (waitpid(container, NULL, WNOHANG)<  0)
>> +        return false;
>>
>>       if (kill(container, 0)<  0&&
>>           errno == ESRCH)
>> @@ -1021,14 +1023,8 @@ cleanup:
>>           VIR_FORCE_CLOSE(loopDevs[i]);
>>       VIR_FREE(loopDevs);
>>
>> -    if (container>  1) {
>> -        int status;
>> -        kill(container, SIGTERM);
>> -        if (!(waitpid(container,&status, WNOHANG) == 0&&
>> -            WIFEXITED(status)))
>> -            kill(container, SIGKILL);
>> -        waitpid(container, NULL, 0);
>> -    }
>> +    virPidAbort(container);
>> +
>>       return rc;
>>   }
>>
>
> This entire method goes away on my patches here
>
> https://www.redhat.com/archives/libvir-list/2011-October/msg00974.html

I guess I'd better review that, then :)

>
> ACK to the other chunks

OK, I split out the lxc_controller fixes (I'm saving them on my tree, 
though, until I know that your patches are in), and pushed the rest.

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