[PATCH] posix: Fix open file action for posix_spawn on Linux
Florian Weimer
fweimer@redhat.com
Mon Sep 19 15:25:00 GMT 2016
On 09/19/2016 04:39 PM, Adhemerval Zanella wrote:
>
>
> On 19/09/2016 09:53, Florian Weimer wrote:
>> On 09/19/2016 02:41 PM, Adhemerval Zanella wrote:
>>
>>> +/* Return if file descriptor is opened. */
>>> +static inline int
>>> +fd_is_valid (int fd)
>>> +{
>>> + return fcntl_not_cancel_2 (fd, F_GETFD) != -1 || errno != EBADF;
>>> +}
>>> +
>>> /* Function used in the clone call to setup the signals mask, posix_spawn
>>> attributes, and file actions. It run on its own stack (provided by the
>>> posix_spawn call). */
>>> @@ -219,6 +226,15 @@ __spawni_child (void *arguments)
>>>
>>> case spawn_do_open:
>>> {
>>> + /* POSIX states that if fildes was already an open file descriptor,
>>> + it shall be closed before the new file is opened. This avoid
>>> + pontential issues when posix_spawn plus addopen action is called
>>> + with the process already at maximum number of file descriptor
>>> + opened and also for multiple actions on single-open special
>>> + paths (like /dev/watchdog). */
>>> + if (fd_is_valid (action->action.open_action.fd))
>>> + close_not_cancel (action->action.open_action.fd);
>>
>> It's not clear to me why you can't just close the file descriptor unconditionally. It does not seem to matter whether you perform fcntl or close on an invalid file descriptor.
>
> 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.
The sysdeps/posix implementation would need a similar change.
However, the new test case consistently fails for me because of the
pipe2 call in the Linux implementation.
Thanks,
Florian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: spawn.patch
Type: text/x-patch
Size: 12997 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20160919/8eb4677e/attachment.bin>
More information about the Libc-alpha
mailing list