[PATCH 3/3] posix: Refactor posix_spawn
Adhemerval Zanella
adhemerval.zanella@linaro.org
Wed Jun 28 12:51:00 GMT 2017
On 27/06/2017 22:07, Rasmus Villemoes wrote:
> 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?
I was in fact unsure about this change and thinking twice now I am not
sure if it indeed adds more readability. My idea was to try share as
much code is possible with the default posix_spawn implementation,
but it ended with a somewhat convoluted code with a lot of #includes.
I am tending to just drop this specific change and just push the
posix implementation (patch 2/3).
>
>>
>> +/* 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
>
More information about the Libc-alpha
mailing list