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 v2 3/3] posix: New Linux posix_spawn{p} implementation



On 14/09/2016 15:58, Rasmus Villemoes wrote:
> On Wed, Sep 14 2016, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> I think patch is ok and fixes the issues you noted about using the pipe2
>> call to signal the execv issue.  It just have one remark about it below.
>>
>>
>>> @@ -280,14 +267,12 @@ __spawni_child (void *arguments)
>>>       (2.15).  */
>>>    maybe_script_execute (args);
>>>
>>> -  ret = -errno;
>>> -
>>>  fail:
>>> -  /* Since sizeof errno < PIPE_BUF, the write is atomic. */
>>> -  ret = -ret;
>>> -  if (ret)
>>> -    while (write_not_cancel (p, &ret, sizeof ret) < 0)
>>> -      continue;
>>> +  /* errno should have an appropriate non-zero value, but make sure
>>> +     that's the case so that our parent knows we failed to
>>> +     exec. There's no EUNKNOWN or EINTERNALBUG, so we use a value
>>> +     which is clearly bogus.  */
>>> +  args->err = errno ? : EHOSTDOWN;
>>>    _exit (SPAWN_ERROR);
>>>  }
>>
>> I would prefer an assert call here to ensure errno is non zero for
>> failure case instead of reporting a bogus errno to program.  Since
>> this unexpected issue is either something wrong being reported from
>> kernel or an underlying bug it would be better to fail at once than
>> instead to document on manuals that this is potentially an unknown
>> issue.
> 
> But asserting/aborting in the child doesn't really solve the problem; we
> still need to write some non-zero value for the parent to pick up once
> we're gone. We could of course write -1 to indicate this really
> exceptional situation, but that still leaves deciding how to handle that
> in the parent. IMO an assert/abort is a little too harsh, but then the
> parent has to return _some_ error code to its caller.

My idea is to in fact not return to parent, but rather terminate program
execution in face of an unknown issue.  However, I do not have a strong
opinion if it should be really the desirable behaviour and thinking twice
it does seems that aborting program is too harsh.  I think -1 would be
suffice.


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