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

[Paul Eggert <eggert at cs dot ucla dot edu>]

Adhemerval Zanella wrote:
> +#define SIGALL_SET ((sigset_t *)(const unsigned long long [2]){ -1,-1 })

This shouldn't assume that _SIGSET_NWORDS is 2. And since the code is 
Linux-specific, you should be able to avoid casting to sigset_t *; just write a 
constant that evaluates to sigset_t * without a cast.

> +#ifdef O_CLOEXEC
> +# ifndef __ASSUME_PIPE2
> +  if (__have_pipe2 >= 0)
> +# endif
> +    {
> +      r = __pipe2 (pipe_fds, O_CLOEXEC);
> +# ifndef __ASSUME_PIPE2
> +      if (__have_pipe2 == 0)
...

This sort of code is hard to read. Instead, declare substitutes like this after 
you do your #includes:

  #ifndef __ASSUME_PIPE2
  # define __have_pipe2 1
  #endif
  #ifndef O_CLOEXEC
  # define O_CLOEXEC 0
  #endif

and let the rest of the code just use __have_pipe2 and O_CLOEXEC, without the 
forest of #ifdefs.

> +struct posix_spawn_args {
> +  int p[2];
> +  sigset_t oldmask;
> +  const char *path;

This is a file name, right? Not a path? If so, it should not be called "path".

> +  int (*exec)(const char *, char *const *, char *const *);

Spaces between )(. Also, please check the indenting overall; it seemed a bit 
inconsistent in this file.

> +      while (argv[argc++]);
...
> +    while (write_not_cancel (p, &ret, sizeof ret) < 0);

Use "continue".

> +      /* Construct an argument list for the shell.  */
> +      char *new_argv[argc];

This can overflow the stack.

> +		  /* Only signal errors for file descriptors out of range.  */

"Signal errors only for ..."