[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