This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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



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).


>>> 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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]