This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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



On 27/09/2016 13:26, Rasmus Villemoes wrote:
> 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
> 

I will commit for you.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]