Re: bin/47524: make(1) loops in Job_CatchOutput()

[David Laight <david%l8s.co.uk@localhost>]

The following reply was made to PR bin/47524; it has been noted by GNATS.

From: David Laight <david%l8s.co.uk@localhost>
To: Andreas Gustafsson <gson%gson.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: bin/47524: make(1) loops in Job_CatchOutput()
Date: Mon, 4 Feb 2013 21:37:07 +0000

 On Mon, Feb 04, 2013 at 09:23:02PM +0200, Andreas Gustafsson wrote:
 > 
 > Can you please explain the reasoning behind the following change you
 > made in job.c 1.122 - why is a failing poll treated as if the
 > childExitJob fd is ready?
 > 
 > -    nready = poll(fds + 1 - wantToken, nfds + 1 - wantToken, POLL_MSEC);
 > -    if (nready <= 0)
 > -       return;
 > +    nready = poll(fds + 1 - wantToken, nfds - 1 + wantToken, POLL_MSEC);
 > 
 > -    if (readyfd(&childExitJob)) {
 > +    if (nready < 0 || readyfd(&childExitJob)) {
 
 I moved the Job_CatchChildren() from being called unconditionally
 everytime around the loop, to be (usually) only called when a
 token was read from the pipe.
 
 But I didn't want to break the code if it was actually relying on the
 poll timeout (5 seconds) to collect children.
 
 > In any case, christos' "fix" of looping five times can't possibly be
 > correct.  The code worked before the loop was added, and should be
 > fixed by removing the loop, not by looping some magic number of times.
 
 I hadn't seen xtos's 'fix' - but it is broken!
 The code could try to read more bytes - would save system calls.
 
        David
 
 -- 
 David Laight: david%l8s.co.uk@localhost