> Date: Thu, 30 Jan 2014 01:16:34 +0400
> From: Pavel Fedin <pavel_fe...@mail.ru>
>
> Hello! This is my long-promised refactor. After this it's much easier
> to apply runtime selection between spawn() and fork() on Cygwin,
> because all differences are now consolidated in two functions:
> child_execute_process() and exec_command().
Thanks.
> In some critical POSIX-specific places i have expanded
> CLOSE_ON_EXEC() macro in order to provide reliable build problem
> tracking. As far as i understand, even very old POSIX systems
> implement this flag, so this should be OK.
This will break the native Windows build, so please don't. (See job.h
for how this macro is defined on Windows.) And even if the places
where you made those expansions are not compiled on Windows, having a
macro in some places and its expansion in others is confusing.
Also, I don't understand this part:
> --- a/job.h
> +++ b/job.h
> @@ -126,13 +126,8 @@ int child_execute_job (char *argv, struct child *child);
> # define FD_STDIN (fileno (stdin))
> # define FD_STDOUT (fileno (stdout))
> # define FD_STDERR (fileno (stderr))
> -# if defined(__EMX__)
> -int child_execute_job (int stdin_fd, int stdout_fd, int stderr_fd,
> +pid_t child_execute_job (int stdin_fd, int stdout_fd, int stderr_fd,
> char **argv, char **envp);
> -# else
> -void child_execute_job (int stdin_fd, int stdout_fd, int stderr_fd,
> - char **argv, char **envp) __attribute__ ((noreturn));
> -# endif
Now systems other than EMX that use 'fork/exec' will not declare
child_execute_job as a function that does not return, which is needed,
since the call to 'exec_command' doesn't return.
And what about this FIXME comment:
> + /* undo FD_CLOEXEC after the child process has been started
> + CHECKME: Is this really needed ? These flags should not harm. */
> if (!(flags & COMMANDS_RECURSE) && job_fds[0] >= 0)
> {
This is based on a first cursory look; I will review the patch some
more in a day or two. (And I hope Paul will as well.)
Thanks again for working on this.
_______________________________________________
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
