[PATCH] linux: spawni.c: simplify error reporting to parent

Rasmus Villemoes rv@rasmusvillemoes.dk
Tue Sep 27 20:26:00 GMT 2016


On Fri, Sep 23 2016, Florian Weimer <fw@deneb.enyo.de> wrote:

> * Adhemerval Zanella:
>
>> On 23/09/2016 16:09, Rasmus Villemoes wrote:
>>> On Fri, Sep 23 2016, Florian Weimer <fw@deneb.enyo.de> wrote:
>>> 
>>>> * Rasmus Villemoes:
>>>>
>>>>> +      ec = args.err;
>>>>> +      assert (ec >= 0);
>>>>> +      if (ec != 0)
>>>>> +	  __waitpid (new_pid, NULL, 0);
>>>>
>>>> One minor issue: Now that the variable name “ec” appears in an
>>>> assertion, it is a good idea to rename it to “error_code_from_child“
>>>> or something similar, so that the assertion message is more
>>>> meaningful.
>>> 
>>> IMO, that's already addressed by the comments above the initialization
>>> in the parent and exit path in the child. An assert message would have
>>> to be overly verbose to make sense without the context of the code it
>>> appears in. In any case, error_code_from_child is a bad name if we hit
>>> the "ec = -new_pid" branch.
>>
>> Also, if CLONE_VM is not really creating a thread with shared VM it means
>> something really broken in the system.
>

I'd be more worried about CLONE_VFORK not being absolutely honoured in
all scenarios (emulation, ptrace, ...).  But that's what we now have the
assert for.

> Agreed.  I have no objections anymore to the patch as-is.

Thanks. Could someone with commit access take this?

Rasmus



More information about the Libc-alpha mailing list