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

[Andreas Gustafsson <gson%gson.org@localhost>]

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

From: Andreas Gustafsson <gson%gson.org@localhost>
To: christos%zoulas.com@localhost (Christos Zoulas)
Cc: dsl%NetBSD.org@localhost,
    gnats-bugs%NetBSD.org@localhost
Subject: Re: bin/47524: make(1) loops in Job_CatchOutput()
Date: Wed, 6 Feb 2013 10:31:21 +0200

 Christos Zoulas wrote:
 > | Index: job.c
 > 
 > The whole logic is fishy. Why is nready decremented when it is not
 > used anymore?
 
 I believe this is vestigial; nready was used after the decrement in
 revision 1.121, for the purpose you allude to in your comment
 "/* Why not use nready here to cut things short? */", but dsl
 removed that shortcut in 1.122, without removing the decrement.
 
 Since this is unrealated to the bug at hand, I felt it was best not to
 clutter my patch with a fix for this but to leave it for a separate
 commit.
 
 > Why treat some descriptors specially and not deal with them in the
 > general way?
 
 If you mean why handle some in the "if (nready > 0) {...}" and others
 after "if (nready <= 0) return;", I agree that it would be cleaner to
 handle them all the same place.  In 1.121, they still were.  Again,
 with my patch, I was not trying to overhaul the entire function, but
 to address the bug at hand with a local change.
 -- 
 Andreas Gustafsson, gson%gson.org@localhost