[2/3] posix: Improve default posix_spawn implementation

  On 29/06/2017 11:22, Andreas Schwab wrote:
> On Mai 15 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> +		    if ((ret = close_not_cancel (new_fd) != 0))
> 
> This assigns the wrong value to ret.

Ugh, this should not be here...

> 
>> +fail:
>> +  /* Since sizeof errno < PIPE_BUF, the write is atomic. */
>> +  ret = -ret;
> 
> posix_spwan is supposed to return errno, but none of the arms going here
> set ret appropriately.

Fixed below.

> 
>> +  /* Generate the new process.  */
>> +  pid_t new_pid = __fork ();
>> +
>> +  if (new_pid == 0)
>> +    __spawni_child (&args);
>> +  else if (new_pid > 0)
>> +    {
>> +      __close (args.pipe[1]);
>> +
>> +      if (__read (args.pipe[0], &ec, sizeof ec) != sizeof ec)
>> +	ec = 0;
>> +      else
>> +	__waitpid (new_pid, &(int) { 0 }, 0);
>>      }
>> -  while (*p++ != '\0');
>> +  else
>> +    ec = -new_pid;
> 
> new_pid isn't an errno either.

Since posix_spawn is not support to set errno in case of failure we will
need to save/restore for fork call.  And this should be fixed on Linux
implementation as well since clone will also clobber errno in case of 
an error (I will send a fix).

Ideally I think the exported clone should not set errno and just return
errno as a negative number.

> 
> Andreas.
> 

I intended to push this change:
  

On Jun 29 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> Since posix_spawn is not support to set errno in case of failure

Is it?

Andreas.
  

On 29/06/2017 12:20, Andreas Schwab wrote:
> On Jun 29 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> Since posix_spawn is not support to set errno in case of failure
> 
> Is it?

My understanding of posix_spawn return value [1] states that the error number 
should be returned as the function return value, not on errno.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/
  

On Jun 29 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> On 29/06/2017 12:20, Andreas Schwab wrote:
>> On Jun 29 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>> 
>>> Since posix_spawn is not support to set errno in case of failure
>> 
>> Is it?
>
> My understanding of posix_spawn return value [1] states that the error number 
> should be returned as the function return value, not on errno.

But that doesn't forbid spurious writes to errno, does it?

Andreas.
  

On 03/07/2017 04:08, Andreas Schwab wrote:
> On Jun 29 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> On 29/06/2017 12:20, Andreas Schwab wrote:
>>> On Jun 29 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>>>
>>>> Since posix_spawn is not support to set errno in case of failure
>>>
>>> Is it?
>>
>> My understanding of posix_spawn return value [1] states that the error number 
>> should be returned as the function return value, not on errno.
> 
> But that doesn't forbid spurious writes to errno, does it?

My understanding is POSIX is not strictly regarding spurious errno writes,
but on general information [1] it has the rationale:

"In order to avoid this problem altogether for new functions, these 
functions avoid using errno and, instead, return the error number directly 
as the function return value; a return value of zero indicates that no 
error was detected."

Also, making 'clone' not setting errno should also a QoI IMHO: this is an
Linux extension and making is avoid touching memory that might incur in
some issue should be less prone to errors.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html
  

On Jul 03 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> My understanding is POSIX is not strictly regarding spurious errno writes,
> but on general information [1] it has the rationale:
>
> "In order to avoid this problem altogether for new functions, these 
> functions avoid using errno and, instead, return the error number directly 
> as the function return value; a return value of zero indicates that no 
> error was detected."

It also notes (in section Signal Actions):

"Note in particular that even the "safe'' functions may modify the global
variable errno"

Andreas.
  

On 03/07/2017 10:02, Andreas Schwab wrote:
> On Jul 03 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> My understanding is POSIX is not strictly regarding spurious errno writes,
>> but on general information [1] it has the rationale:
>>
>> "In order to avoid this problem altogether for new functions, these 
>> functions avoid using errno and, instead, return the error number directly 
>> as the function return value; a return value of zero indicates that no 
>> error was detected."
> 
> It also notes (in section Signal Actions):
> 
> "Note in particular that even the "safe'' functions may modify the global
> variable errno"

So it is more an implementation detail and/or QoI to whether modify errno
on such functions and see no impeding reason of doing it on posix_spawn.
The only rationale I am not sure is if its worth to remove clone wrapper
error path setting errno.