[PATCH 2/2] posix: New Linux posix_spawn{p} implementation
Paul Eggert
eggert@cs.ucla.edu
Wed Jan 27 16:17:00 GMT 2016
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 ..."
More information about the Libc-alpha
mailing list