[PATCH] posix: Fix open file action for posix_spawn on Linux
Florian Weimer
fw@deneb.enyo.de
Tue Sep 20 20:16:00 GMT 2016
* 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.
More information about the Libc-alpha
mailing list