[PATCH 3/3] posix: New Linux posix_spawn{p} implementation

[Paul Eggert]

Similar issues with int vs ptrdiff_t, argc + 1, etc.

This patch creates new lines with trailing white space; please avoid 
that. (Doesn't git complain about that to you? It does to me.)

>   /* To avoid impose hard limits on posix_spawn{p} total number of 
> arguments

impose -> imposing
total -> the total

      valus.  */

values

>   /* Add a slack area to child own stack. */

child -> the child's

>    clobber due stack spilling). The remaning issue are:

Misspelling of "remaining".

>    1. That no signal handlers must run in child context, to avoid corrupt

corrupt -> corrupting

>    a pipe or waitpid (in case or error). The pipe has the advantage of
>    allowing the child the signal an exec error.  */

the signal -> to signal (or better yet, reword to avoid "signal" since 
this doesn't use signals)

>   /* Then apply FD_CLOCEXEC if it is supported in the pipe call case.  */

Misspelled "FD_CLOEXEC".

>   /* Try pipe2 even if __ASSUME_PIPE2 is not define and returning an error
>      iff the call returns ENOSYS.  */
define and returning -> defined and return

>       if (__have_pipe2 > 0)
>     return r;

This code is unnecessary and can be removed.

>   if (__have_pipe2 < 0)
>     if (__pipe (pipe_fds) < 0)
>       return -1;
>
>   /* Then apply FD_CLOCEXEC if it is supported in the pipe call case.  */
>   if (__have_pipe2 < 0)
>     {
>       if ((r = __fcntl (pipe_fds[0], F_SETFD, FD_CLOEXEC)) == -1
>       || (r = __fcntl (pipe_fds[1], F_SETFD, FD_CLOEXEC)) == -1)
>     {
>       close_not_cancel (pipe_fds[0]);
>       close_not_cancel (pipe_fds[1]);
>       return r;
>     }
>     }

This tests __have_pipe2 twice, whereas it should test it just once.

    to be called explicity using /bin/sh (_PATH_BSHELL).  */

Misspelled "explicitly".

> {
>   if (xflags & SPAWN_XFLAGS_USE_PATH)
>     return __spawnix (pid, file, acts, attrp, argv, envp, xflags, 
> __execvpe);
>   return __spawnix (pid, file, acts, attrp, argv, envp, xflags, __execve);
> }

Put the if-then-else inside the __spawnix call, to avoid repetition.  
Something like:

    return __spawnix (pid, file, acts, attrp, argv, envp, xflags,
                                 xflags & SPAWN_XFLAGS_USE_PATH ? 
__execvpe : __execve);

>   const int prot = (PROT_READ | PROT_WRITE
>             | ((GL (dl_stack_flags) & PF_X) ? PROT_EXEC : 0));
>
>   /* Add a slack area to child own stack.  */
>   const size_t argv_size = (argc * sizeof (void *)) + 512;
>   const size_t stack_size = ALIGN_UP (argv_size, GLRO(dl_pagesize));

There is typically no need for locals to be declared "const". We can 
easily see that they are not changed by reading the code, and the 
"const" makes the declaration a bit harder to read.