Re: [PATCH] posix: Fix open file action for posix_spawn on Linux

[Florian Weimer <fw at deneb dot enyo dot de>]

* Adhemerval Zanella:

> On 20/09/2016 16:57, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>>>> Indeed it seems superfluous to check for file descriptor validity before
>>>>> calling close, I will remove it.  With this changes, do you have any
>>>>> pending issues for the patch?
>>>>
>>>> I'd rather suggest something like this, avoiding the close in the
>>>> common case when the open call succeeds.
>>>
>>> This approach has the issue that to 1. potentially trigger the dup + close
>>> case more often because the new fd won't be equal to the expected one,
>>> while closing the file descriptor first might avoid this issue; and 2.
>>> to always trigger a failure in the behaviour described in the patch
>>> (EMFILE).
>> 
>> 1. seems a valid argument to me.  It may pollute the strace output in
>> some cases, but it's true that it's a net savings in the most common
>> cases. Suggestion withdrawn.
>> 
>> 2. I do not understand.
>> 
>
> Issuing open and check its result in a program that reached its maximum
> file descriptor count (EMFILE situation such as the testcase aims to 
> reproduce) will always trigger the exception case where it will require 
> to close and retry the open (so instead of just close plus open, it will
> be open failure, close, open).

Yes, but this case will be exceedingly rare.

>>>> However, the new test case consistently fails for me because of the
>>>> pipe2 call in the Linux implementation.
>>>
>>> Sorry, I should have stated that it requires the pipe removal
>>> patch [1] on posix_spawn.
>>>
>>> [1] https://sourceware.org/ml/libc-alpha/2016-09/msg00006.html
>> 
>> Okay, we'll have to get that patch in first obviously.
>> 
>
> I think we just need to align which would be best error option for
> unknown issues that do not set the errno (because the error code
> relies on errno value).  Original patch suggested returning a
> bogus value (EHOSTDOWN), where I would prefer just -1 to avoid
> implement specific behaviour.

But we have to set a different errno value eventually, for the
application to read.  Let's continue that discussion on the other
thread.