Re: [PATCH 1/4] run-command: add new check_command helper

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Apr 2, 2013, 05:22 UTC
Message-ID: <CAMP44s3oVJs_sWpemFVGSkj-u_SrjsxE1mYja+Scx3PTBXC67Q@mail.gmail.com>
In-Reply-To: <20130402051407.GA21906@sigill.intra.peff.net>

On Mon, Apr 1, 2013 at 11:14 PM, Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

> On Mon, Apr 01, 2013 at 11:11:20PM -0600, Felipe Contreras wrote:
>
>> > But if we know from reading waitpid(3) that waitpid should only fail due
>> > to EINTR, or due to bogus arguments (e.g., a pid that does not exist or
>> > has already been reaped), then maybe something like this makes sense:
>> >
>> >   while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
>> >           ; /* nothing */
>>
>> But we don't want to wait synchronously here, we just want to ping.
>
> Yeah, sorry, I forgot the WNOHANG there.

It still can potentially stay in a loop for some cycles.

Show 14 quoted lines

>> > After the fix above, yes; in the original we would always have exited
>> > already.
>>
>> No:
>>
>> +       if (waiting != cmd->pid)
>> +               return 1;
>>
>> If waiting < 0, waiting != cmd->pid, and therefore this return is not
>> triggered, and there's only one more return at the end of the
>> function.
>
> Are my eyes not working? If waiting < 0, then waiting != cmd->pid, and
> therefore this return _is_ triggered.

Oh, right, it's only after the modification that the code works.

-- 
Felipe Contreras