Re: [PATCH v2 3/3] posix: New Linux posix_spawn{p} implementation

[Rasmus Villemoes <rv@rasmusvillemoes.dk>]

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.

Rasmus