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

[Rasmus Villemoes <rv@rasmusvillemoes.dk>]

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

> * Rasmus Villemoes:
>
>> +  /* 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 think ECHILD is probably a better fake error code.

Yeah, that's probably ok. It's consistent with its use in pclose() where
the posix description reads "The status of the child process could not
be obtained". We just use it in the sense "something went wrong, we just
don't know what".

I'd really wish EINTERNALBUG existed.

> You should set args->err to 0 on success ...
>
>> +  args.err = 0;
>
> ... and initialize it to -1.
>
>>    if (new_pid > 0)
>>      {
>> -      if (__read (args.pipe[0], &ec, sizeof ec) != sizeof ec)
>> -    ec = 0;
>> -      else
>> +      ec = args.err;
>> +      if (ec != 0)
>>      __waitpid (new_pid, NULL, 0);
>>      }
>
> You should check (assert?) here that args.err is not -1.  Otherwise we
> will never notice if the page is not shared between parent and child,
> and the error reporting mechanism does not work.

Good point. I'll send an updated patch in a moment.

Rasmus