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.
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:pavel_fe...@mail.ru
make-merge-child_execute_job-code.diff
Description: Binary data
_______________________________________________ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
