[PATCH] Refactor and merge child_execute_job() code, new attempt

[Pavel Fedin]

Hello, Paul! Sorry for so long delay, i'm really quite busy, however i
have  found  some  time  to  get  back  to this. Please review the new
version.

 This  is actually a repost. I have posted this message a week ago and
got no response. I suggest it fell into old thread, so you have missed
it.

Monday, February 3, 2014, 0:30:26 you wrote:

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

 Done.

> 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.

 Yes.  This  function  does  very OS-specific things, and personally i
don't  see  how it could be done in some other way. Well, except if we
completely  split  it  up  and introduce some new OS-specific .c files
which  will  hold  implementations of OS-specific functions.

> 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.

 Done.  After  some  code review i have noticed that we actually don't
need   this  at all when using fork(), because execvp() is executed in
child context. So only spawn() code actually uses it now.

> 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.

 Yes,  we  can  do  also this. However perhaps this would not look too
good  because  we  call child_execute_job() from two different places,
and  we  need  to deal with these fd's only in one place. I think it's
not a big loss to reset the flag.

-- 
С уважением,
 Pavel                          mailto:address@hidden

make-merge-child_execute_job-code.diff
Description: Binary data