Re: [PATCH] Refactor and merge child_execute_job() code

[Paul Smith]

On Fri, 2014-01-31 at 10:32 +0400, Pavel Fedin wrote:
> Ok, i actually can leave it as a macro.

I prefer the macro form as well; please keep it that way.

Also, I'm not sure I like the current child_execute_job() where there
are two completely different implementations in the same function,
handled by ifdef.  But I guess we do the same thing in other places.

Other things:

Can you push down the environ pointer resetting into exec_command(),
rather than having the save/restore in start_job_command()?  We don't
need this on POSIX systems so it would be nice if it was not done there.

For the CHECKME stuff, we do need to keep the CLOEXEC reset.  Those file
descriptors definitely must be passed down to the child if the child is
a make process so that that make process can handle its jobserver
duties.  That's what the COMMANDS_RECURSE flag is for.  At least this is
true for POSIX systems.  I think the Windows implementation uses a
semaphore instead of a pipe so it doesn't need to worry about these.

However it seems we might be able to simplify some things, especially on
POSIX systems, by pushing the handling of it down into
child_execute_job().  To do that we'd need to pass the flags argument to
child_execute_job(), or at least a boolean value specifying whether
COMMANDS_RECURSE is set.  But then on POSIX systems, at least, we could
do the close-on-exec in the child's fork and we wouldn't have to undo
it.