bash: Correct usage of F_SETFD

bash: Correct usage of F_SETFD

[Sukadev Bhattiprolu]

include/filecntl.h in bash-4.1 has following:

#define SET_CLOSE_ON_EXEC(fd)  (fcntl ((fd), F_SETFD, FD_CLOEXEC))

Is that really the correct/intended usage of F_SETFD ?

If kernel ever adds a new flag to the fd, this would end up clearing the
other new flag right ?

Shouldn't bash use F_GETFD to get the current flags and set/clear just
the FD_CLOEXEC bit ?

Sukadev

Re: bash: Correct usage of F_SETFD

[Chet Ramey]

> include/filecntl.h in bash-4.1 has following:
> 
> #define SET_CLOSE_ON_EXEC(fd)  (fcntl ((fd), F_SETFD, FD_CLOEXEC))
> 
> Is that really the correct/intended usage of F_SETFD ?

     F_SETFD            Set the close-on-exec flag associated with fildes to
                        the low order bit of arg (0 or 1 as above).

> If kernel ever adds a new flag to the fd, this would end up clearing the
> other new flag right ?
> 
> Shouldn't bash use F_GETFD to get the current flags and set/clear just
> the FD_CLOEXEC bit ?

I suppose it would matter if there are systems that have more than one
flag value.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/

Re: bash: Correct usage of F_SETFD

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

On 11/22/2010 03:16 PM, Chet Ramey wrote:
>> include/filecntl.h in bash-4.1 has following:
>>
>> #define SET_CLOSE_ON_EXEC(fd)  (fcntl ((fd), F_SETFD, FD_CLOEXEC))
>>
>> Is that really the correct/intended usage of F_SETFD ?
> 
>      F_SETFD            Set the close-on-exec flag associated with fildes to
>                         the low order bit of arg (0 or 1 as above).
> 
>> If kernel ever adds a new flag to the fd, this would end up clearing the
>> other new flag right ?
>>
>> Shouldn't bash use F_GETFD to get the current flags and set/clear just
>> the FD_CLOEXEC bit ?
> 
> I suppose it would matter if there are systems that have more than one
> flag value.

In practice, there aren't any such systems; but POSIX warns that current
practice is no indicator of future systems, and that read-modify-write
is the only way to use F_SETFD.

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: bash: Correct usage of F_SETFD

[Sukadev Bhattiprolu]

Eric Blake [eblake@redhat.com] wrote:
| On 11/22/2010 03:16 PM, Chet Ramey wrote:
| >> include/filecntl.h in bash-4.1 has following:
| >>
| >> #define SET_CLOSE_ON_EXEC(fd)  (fcntl ((fd), F_SETFD, FD_CLOEXEC))
| >>
| >> Is that really the correct/intended usage of F_SETFD ?
| > 
| >      F_SETFD            Set the close-on-exec flag associated with fildes to
| >                         the low order bit of arg (0 or 1 as above).

Is that the POSIX definition ? Following man page does not limit F_SETFD to
FD_CLOEXEC:

	http://www.kernel.org/doc/man-pages/online/pages/man2/fcntl.2.html

	F_SETFD (long)
		    Set the file descriptor flags to the value specified by arg.
| > 
| >> If kernel ever adds a new flag to the fd, this would end up clearing the
| >> other new flag right ?
| >>
| >> Shouldn't bash use F_GETFD to get the current flags and set/clear just
| >> the FD_CLOEXEC bit ?
| > 
| > I suppose it would matter if there are systems that have more than one
| > flag value.
| 
| In practice, there aren't any such systems; but POSIX warns that current
| practice is no indicator of future systems, and that read-modify-write
| is the only way to use F_SETFD.

Yes, that seems to make more sense.

Sukadev

Re: bash: Correct usage of F_SETFD

[Matthew Wilcox]

On Mon, Nov 22, 2010 at 04:04:46PM -0800, Sukadev Bhattiprolu wrote:
> Eric Blake [eblake@redhat.com] wrote:
> | On 11/22/2010 03:16 PM, Chet Ramey wrote:
> | >> include/filecntl.h in bash-4.1 has following:
> | >>
> | >> #define SET_CLOSE_ON_EXEC(fd)  (fcntl ((fd), F_SETFD, FD_CLOEXEC))
> | >>
> | >> Is that really the correct/intended usage of F_SETFD ?
> | > 
> | >      F_SETFD            Set the close-on-exec flag associated with fildes to
> | >                         the low order bit of arg (0 or 1 as above).
> 
> Is that the POSIX definition ? Following man page does not limit F_SETFD to
> FD_CLOEXEC:
> 
> 	http://www.kernel.org/doc/man-pages/online/pages/man2/fcntl.2.html
> 
> 	F_SETFD (long)
> 		    Set the file descriptor flags to the value specified by arg.

The POSIX definition can be found here:
http://www.opengroup.org/onlinepubs/009695399/functions/fcntl.html

> | In practice, there aren't any such systems; but POSIX warns that current
> | practice is no indicator of future systems, and that read-modify-write
> | is the only way to use F_SETFD.
> 
> Yes, that seems to make more sense.

I think future flags will be created such that they default to off,
and bash would have to affirmitively set them in order to use them.

So if bash is the one creating its file descriptors, there's no need to
use R/M/W since it knows what the state of them are.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: bash: Correct usage of F_SETFD

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

On 11/23/2010 07:42 AM, Matthew Wilcox wrote:
> The POSIX definition can be found here:
> http://www.opengroup.org/onlinepubs/009695399/functions/fcntl.html
> 
>> | In practice, there aren't any such systems; but POSIX warns that current
>> | practice is no indicator of future systems, and that read-modify-write
>> | is the only way to use F_SETFD.
>>
>> Yes, that seems to make more sense.
> 
> I think future flags will be created such that they default to off,
> and bash would have to affirmitively set them in order to use them.

Not true.  An implementation can reasonably define a new flag to off for
backwards-compatible behavior, and on for POSIX-compatible behavior, if
there is a case where traditional and POSIX behavior differ.  POSIX
permits additional bits to be on, and in fact requires that applications
leave those additional bits unchanged, in the very case where those
additional bits are essential for maintaining a POSIX-compatible
environment.

> 
> So if bash is the one creating its file descriptors, there's no need to
> use R/M/W since it knows what the state of them are.

No, bash cannot reasonably know what the implementation's default bit
state is, and blindly setting all other bits to zero is very possibly a
bug, and easy enough to avoid by using the full R/M/W.

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: bash: Correct usage of F_SETFD

[Sukadev Bhattiprolu]

Eric Blake [eblake@redhat.com] wrote:
| > 
| > So if bash is the one creating its file descriptors, there's no need to
| > use R/M/W since it knows what the state of them are.
| 
| No, bash cannot reasonably know what the implementation's default bit
| state is, and blindly setting all other bits to zero is very possibly a
| bug, and easy enough to avoid by using the full R/M/W.

Besides, if user sets a flag on the fd and execs bash, bash would clobber
the flag. IOW, assuming that bash is the one creating the fd is not always
safe right ?

Thanks,

Sukadev

Re: bash: Correct usage of F_SETFD

[Jamie Lokier]

Eric Blake wrote:
> On 11/22/2010 03:16 PM, Chet Ramey wrote:
> >> include/filecntl.h in bash-4.1 has following:
> >>
> >> #define SET_CLOSE_ON_EXEC(fd)  (fcntl ((fd), F_SETFD, FD_CLOEXEC))
> >>
> >> Is that really the correct/intended usage of F_SETFD ?
> > 
> >      F_SETFD            Set the close-on-exec flag associated with fildes to
> >                         the low order bit of arg (0 or 1 as above).
> > 
> >> If kernel ever adds a new flag to the fd, this would end up clearing the
> >> other new flag right ?
> >>
> >> Shouldn't bash use F_GETFD to get the current flags and set/clear just
> >> the FD_CLOEXEC bit ?
> > 
> > I suppose it would matter if there are systems that have more than one
> > flag value.
> 
> In practice, there aren't any such systems; but POSIX warns that current
> practice is no indicator of future systems, and that read-modify-write
> is the only way to use F_SETFD.

There are so many programs using F_SETFD the way Bash does, that it
would be quite brave (or stupid) to add another flag.

-- Jamie