Re: [PATCH 3/3] posix: Refactor posix_spawn

[Rasmus Villemoes <rv at rasmusvillemoes dot dk>]

On Mon, May 15 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> This patch refactor both POSIX and Linux internal posix_spawn to use
> a common implementation.  The POSIX one is parametrized to use define
> hooks and Linux reimplements it on its file.  No functional change is
> expected.
>

Hm, I do think this makes the code quite a bit harder to understand,
since one has to skip back and forth between the #includer and the file
with the bulk of the implementation to see what actually happens.

I didn't read it all carefully, but there were at least a number of places
where your macro parameters have a double leading underscore, but then
you omit those in the macro body. I don't suppose that's intentional?

>  
> +/* errno should have an appropriate non-zero value; otherwise,
> +   there's a bug in glibc or the kernel.  For lack of an error code
> +   (EINTERNALBUG) describing that, use ECHILD.  Another option would
> +   be to set args->err to some negative sentinel and have the parent
> +   abort(), but that seems needlessly harsh.  */
> +#define _POSIX_SPAWN_CHILD_SYNC_END(__ret, __args) \
> +  args->err = errno ? : ECHILD

here

> +struct posix_spawn_args;
> +static int __spawn_process (pid_t *pid, ptrdiff_t argc,
> +			    struct posix_spawn_args *args);
> +#define _POSIX_SPAWN_PARENT_PROCESS(__pid, __argc, __args) \
> +  __spawn_process (__pid, __argc, __args)
> +
> +#define _POSIX_SPAWN_PARENT_BLOCK_SIGNALS(__args) \
> +  __libc_signal_block_all (&__args.oldmask);
> +
> +#define _POSIX_SPAWN_PARENT_RESTORE_SIGNALS(__args) \
> +  __libc_signal_restore_set (&args.oldmask);

here