Al,
your 2008-07-26 commit 6c5d0512a091480c9f981162227fdb1c9d70e555 for
fs/fcntl.c:sys_dup3 contains
+ if (unlikely(oldfd == newfd))
+ return -EINVAL;
This makes dup2() and dup3() differ with respect to the oldfd==newfd
case (dup2() becomes a no-op, just returning oldfd, dup3() gives the
EINVAL error.
Your commit log doesn't explain the rationale for this change. What
is it? (I could guess that it is to error on a case that may be a
user programming error, but I want to check this with you, so that I
add the right text to the man page.)
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
Michael Kerrisk wrote:
> Al,
>
> your 2008-07-26 commit 6c5d0512a091480c9f981162227fdb1c9d70e555 for
> fs/fcntl.c:sys_dup3 contains
>
> + if (unlikely(oldfd == newfd))
> + return -EINVAL;
>
> This makes dup2() and dup3() differ with respect to the oldfd==newfd
> case (dup2() becomes a no-op, just returning oldfd, dup3() gives the
> EINVAL error.
>
> Your commit log doesn't explain the rationale for this change. What
> is it? (I could guess that it is to error on a case that may be a
> user programming error, but I want to check this with you, so that I
> add the right text to the man page.)
>
The dup2() behavior comes from the logical consequence of dup2()'s
"close on reuse"; one would think it would be logical for dup3() to
behave the same way.
-hpa
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
H. Peter Anvin wrote:
> The dup2() behavior comes from the logical consequence of dup2()'s
> "close on reuse"; one would think it would be logical for dup3() to
> behave the same way.
No. We deliberately decided on this change. Otherwise, what is the
result of dup3(fd, fd, O_CLOEXEC)? There is no reason to use
dup2(fd,fd), so why the hell somebody wants to defend this is beyond me.
- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iEYEARECAAYFAkjuaisACgkQ2ijCOnn/RHRBBgCeMtzyHtpv7jt5a2XxIq9LEoDN
ZVYAnixMwtW6d6SL55MvrKwV/B5Yv1Cm
=MCqO
-----END PGP SIGNATURE-----
Ulrich Drepper wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> H. Peter Anvin wrote:
>> The dup2() behavior comes from the logical consequence of dup2()'s
>> "close on reuse"; one would think it would be logical for dup3() to
>> behave the same way.
>
> No. We deliberately decided on this change. Otherwise, what is the
> result of dup3(fd, fd, O_CLOEXEC)? There is no reason to use
> dup2(fd,fd), so why the hell somebody wants to defend this is beyond me.
>
The result of dup3(fd, fd, O_CLOEXEC) is to set the O_CLOEXEC flag on fd.
The behaviour of dup2() is functionally the following:
1. Duplicate the file descriptor from file_table[oldfd].
2. If file_table[newfd] is in use, close it.
3. Install the duplicate file descriptor at file_table[newfd].
Step (2) could be considered a bit dubious, but the behaviour of
dup2(fd, fd) is a direct consequence of the chosen semantics.
-hpa
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
H. Peter Anvin wrote:
> The result of dup3(fd, fd, O_CLOEXEC) is to set the O_CLOEXEC flag on fd.
That's bad and disregarded by Al and myself because it is one and the
same descriptor and therefore it changes the source descriptor.
> Step (2) could be considered a bit dubious, but the behaviour of
> dup2(fd, fd) is a direct consequence of the chosen semantics.
The behavior of dup2(fd,fd) is just a result of an accident in the
original implementation. It makes no sense and the mistake doesn't have
to be repeated.
- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iEYEARECAAYFAkjubyIACgkQ2ijCOnn/RHRIMwCfdFeW08lSPRh12C+qKzF99AWf
idEAn0x0jqcVEIzmcgIVuZLlHKleNmC0
=Dx/d
-----END PGP SIGNATURE-----
Ulrich Drepper wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> H. Peter Anvin wrote:
>> The result of dup3(fd, fd, O_CLOEXEC) is to set the O_CLOEXEC flag on fd.
>
> That's bad and disregarded by Al and myself because it is one and the
> same descriptor and therefore it changes the source descriptor.
It's not the source descriptor, per se, it is the "new" descriptor,
which happens to have a side effect of closing the "old" descriptor.
>> Step (2) could be considered a bit dubious, but the behaviour of
>> dup2(fd, fd) is a direct consequence of the chosen semantics.
>
> The behavior of dup2(fd,fd) is just a result of an accident in the
> original implementation. It makes no sense and the mistake doesn't have
> to be repeated.
Inconsistency is bad, too, and one could *definitely* argue that the
fundamental problem is the one of closing a pre-existing descriptor
rather than forcing the user to do that explicitly if that behaviour was
desired.
-hpa
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
H. Peter Anvin wrote:
> Inconsistency is bad, too, and one could *definitely* argue that the
> fundamental problem is the one of closing a pre-existing descriptor
> rather than forcing the user to do that explicitly if that behaviour was
> desired.
This is a new interface. It's not breaking any code and fixing problems
with older interfaces is exactly what you do in such situations.
- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iEYEARECAAYFAkjucdAACgkQ2ijCOnn/RHQJ2gCgm3vuOgcPJPl3RgdehydbQjkc
2BgAoIM8KBKh94ge2IVBVmqknWs4hwEk
=2KCr
-----END PGP SIGNATURE-----
On Thu, Oct 9, 2008 at 10:31 PM, Ulrich Drepper <[email protected]> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> H. Peter Anvin wrote:
>> The dup2() behavior comes from the logical consequence of dup2()'s
>> "close on reuse"; one would think it would be logical for dup3() to
>> behave the same way.
>
> No. We deliberately decided on this change. Otherwise, what is the
> result of dup3(fd, fd, O_CLOEXEC)?
Good point. However, I don't find any list mail that discussed this
implementation choice or its rationale. That would have been useful.
> There is no reason to use
> dup2(fd,fd), so why the hell somebody wants to defend this is beyond me.
There is no defense, except consistency with historical behavior. My
reasons for raising this thread were:
a) to point out the inconsistency with dup2().
b) to check that the change was intentional (though it was already
fairly clear that it was)
c) to determine the reason(s) for the change (which were not made clear)
As you point out, we have various possibilities with dup3(fd, fd, flags)
1) Do nothing to fd.
2) change the O_CLOEXEC flag for fd.
3) Give an error.
The problem is that depending on one's perspective one argue that
either 1 or 2 is consistent with dup2(), so 3 seems a reasonable thing
to do.
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
On Thu, Oct 9, 2008 at 10:57 PM, H. Peter Anvin <[email protected]> wrote:
> Ulrich Drepper wrote:
>>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> H. Peter Anvin wrote:
>>>
>>> The result of dup3(fd, fd, O_CLOEXEC) is to set the O_CLOEXEC flag on fd.
>>
>> That's bad and disregarded by Al and myself because it is one and the
>> same descriptor and therefore it changes the source descriptor.
>
> It's not the source descriptor, per se, it is the "new" descriptor, which
> happens to have a side effect of closing the "old" descriptor.
>
>>> Step (2) could be considered a bit dubious, but the behaviour of
>>> dup2(fd, fd) is a direct consequence of the chosen semantics.
>>
>> The behavior of dup2(fd,fd) is just a result of an accident in the
>> original implementation. It makes no sense and the mistake doesn't have
>> to be repeated.
>
> Inconsistency is bad, too, and one could *definitely* argue that the
> fundamental problem is the one of closing a pre-existing descriptor rather
> than forcing the user to do that explicitly if that behaviour was desired.
Well, as long as we are fixing the dup3() interface in the way that Al
and Ulrich have suggested, what about another fix:
give an error if newfd is already open, thus forcing the user to do an
explicit close
?
This silent close in dup2() is an implementation blemish. Why not eliminate it?
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
Michael Kerrisk <[email protected]> wrote:
> Well, as long as we are fixing the dup3() interface in the way that Al
> and Ulrich have suggested, what about another fix:
>
> give an error if newfd is already open, thus forcing the user to do an
> explicit close
>
> ?
>
> This silent close in dup2() is an implementation blemish. Why not eliminate
> it?
I think it might be usefull:
Thread B does some logging to fd 42
Thread A switches the logfile by creating a new file, writing a header and
then does dup3(fd, 42, O_WRONLY|O_APPEND|O_CLOEXEC); close(fd);
(Off cause this is not yet implemented, O_RDONLY would give some problems,
O_CLOEXEC alone might be better done while open()ing the file, ... but you
get the idea.)
BTW: I think dup3(fd, -1, flags) should use the file descriptor dup() would
return. Or should there be a dupf(fd, flags) syscall instead?
On Fri, Oct 10, 2008 at 1:42 PM, Bodo Eggert <[email protected]> wrote:
> Michael Kerrisk <[email protected]> wrote:
>
>> Well, as long as we are fixing the dup3() interface in the way that Al
>> and Ulrich have suggested, what about another fix:
>>
>> give an error if newfd is already open, thus forcing the user to do an
>> explicit close
>>
>> ?
>>
>> This silent close in dup2() is an implementation blemish. Why not eliminate
>> it?
>
> I think it might be usefull:
> Thread B does some logging to fd 42
> Thread A switches the logfile by creating a new file, writing a header and
> then does dup3(fd, 42, O_WRONLY|O_APPEND|O_CLOEXEC); close(fd);
I don't know the details of the kernel locks here, so perhaps this is
a naive question: but, as things stand is there not the potential for
some nasty race if one thread is writing to fd 42 at the same time as
another thread does a dup2(fd, 42)?
> (Off cause this is not yet implemented, O_RDONLY would give some problems,
> O_CLOEXEC alone might be better done while open()ing the file, ... but you
> get the idea.)
>
>
> BTW: I think dup3(fd, -1, flags) should use the file descriptor dup() would
> return. Or should there be a dupf(fd, flags) syscall instead?
If one did this, maybe it would be better to have an extra flag that
said: "use the first free file descriptor >= newfd", thus giving the
more general functionality like fcntl(F_DUPFD).
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
On Fri, 2008-10-10 at 07:04 +0200, Michael Kerrisk wrote:
[....]
> Well, as long as we are fixing the dup3() interface in the way that Al
> and Ulrich have suggested, what about another fix:
>
> give an error if newfd is already open, thus forcing the user to do an
> explicit close
>
> ?
>
> This silent close in dup2() is an implementation blemish. Why not eliminate it?
Apart from the usual "do not break almost all existing apps" killer
reason: The alternative is that people will simply add a "close(newfd)"
everytime before "dup2(oldfd,newfd)" since close() is harmless on a
non-open fd.
Bernd
--
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and Services
On Fri, Oct 10, 2008 at 2:09 PM, Bernd Petrovitsch <[email protected]> wrote:
> On Fri, 2008-10-10 at 07:04 +0200, Michael Kerrisk wrote:
> [....]
>> Well, as long as we are fixing the dup3() interface in the way that Al
>> and Ulrich have suggested, what about another fix:
>>
>> give an error if newfd is already open, thus forcing the user to do an
>> explicit close
>>
>> ?
>>
>> This silent close in dup2() is an implementation blemish. Why not eliminate it?
>
> Apart from the usual "do not break almost all existing apps" killer
> reason: The alternative is that people will simply add a "close(newfd)"
> everytime before "dup2(oldfd,newfd)" since close() is harmless on a
> non-open fd.
Bernd,
I think you've missed the point. The idea is not to change to dup2(),
but to eliminate the blemish in its design in the new dup3() (since we
have alrady eliminated one other blemish).
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
On Fri, 2008-10-10 at 14:15 +0200, Michael Kerrisk wrote:
> On Fri, Oct 10, 2008 at 2:09 PM, Bernd Petrovitsch <[email protected]> wrote:
> > On Fri, 2008-10-10 at 07:04 +0200, Michael Kerrisk wrote:
> > [....]
> >> Well, as long as we are fixing the dup3() interface in the way that Al
> >> and Ulrich have suggested, what about another fix:
> >>
> >> give an error if newfd is already open, thus forcing the user to do an
> >> explicit close
> >>
> >> ?
> >>
> >> This silent close in dup2() is an implementation blemish. Why not eliminate it?
> >
> > Apart from the usual "do not break almost all existing apps" killer
> > reason: The alternative is that people will simply add a "close(newfd)"
> > everytime before "dup2(oldfd,newfd)" since close() is harmless on a
> > non-open fd.
>
> Bernd,
>
> I think you've missed the point. The idea is not to change to dup2(),
That may well be. So the "eliminate it" apparently doesn't mean
"eliminate it in dup2()".
> but to eliminate the blemish in its design in the new dup3() (since we
> have alrady eliminated one other blemish).
FWIW I consider the automatic close() in dup2() a feature (if only that
it avoids an additional system call). So I would include it in dup3()
too.
But a sane, clear and consistent definition and handling of flags are
more important the auto-close() feature.
Bernd
--
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and Services
On Fri, 10 Oct 2008, Michael Kerrisk wrote:
> On Fri, Oct 10, 2008 at 1:42 PM, Bodo Eggert <[email protected]> wrote:
> > Michael Kerrisk <[email protected]> wrote:
> >
> >> Well, as long as we are fixing the dup3() interface in the way that Al
> >> and Ulrich have suggested, what about another fix:
> >>
> >> give an error if newfd is already open, thus forcing the user to do an
> >> explicit close
> >>
> >> ?
> >>
> >> This silent close in dup2() is an implementation blemish. Why not eliminate
> >> it?
> >
> > I think it might be usefull:
> > Thread B does some logging to fd 42
> > Thread A switches the logfile by creating a new file, writing a header and
> > then does dup3(fd, 42, O_WRONLY|O_APPEND|O_CLOEXEC); close(fd);
>
> I don't know the details of the kernel locks here, so perhaps this is
> a naive question: but, as things stand is there not the potential for
> some nasty race if one thread is writing to fd 42 at the same time as
> another thread does a dup2(fd, 42)?
I strongly hope there would not be any ...
> > (Off cause this is not yet implemented, O_RDONLY would give some problems,
> > O_CLOEXEC alone might be better done while open()ing the file, ... but you
> > get the idea.)
> >
> >
> > BTW: I think dup3(fd, -1, flags) should use the file descriptor dup() would
> > return. Or should there be a dupf(fd, flags) syscall instead?
>
> If one did this, maybe it would be better to have an extra flag that
> said: "use the first free file descriptor >= newfd", thus giving the
> more general functionality like fcntl(F_DUPFD).
I think you are right. I thought about something similar, too, but was
distracted enough to not think about the connection.
--
Funny quotes:
10. Nothing is fool proof to a talented fool.
On Fri, Oct 10, 2008 at 3:02 PM, Bernd Petrovitsch <[email protected]> wrote:
> On Fri, 2008-10-10 at 14:15 +0200, Michael Kerrisk wrote:
>> On Fri, Oct 10, 2008 at 2:09 PM, Bernd Petrovitsch <[email protected]> wrote:
>> > On Fri, 2008-10-10 at 07:04 +0200, Michael Kerrisk wrote:
>> > [....]
>> >> Well, as long as we are fixing the dup3() interface in the way that Al
>> >> and Ulrich have suggested, what about another fix:
>> >>
>> >> give an error if newfd is already open, thus forcing the user to do an
>> >> explicit close
>> >>
>> >> ?
>> >>
>> >> This silent close in dup2() is an implementation blemish. Why not eliminate it?
>> >
>> > Apart from the usual "do not break almost all existing apps" killer
>> > reason: The alternative is that people will simply add a "close(newfd)"
>> > everytime before "dup2(oldfd,newfd)" since close() is harmless on a
>> > non-open fd.
>>
>> Bernd,
>>
>> I think you've missed the point. The idea is not to change to dup2(),
>
> That may well be. So the "eliminate it" apparently doesn't mean
> "eliminate it in dup2()".
Right.
>> but to eliminate the blemish in its design in the new dup3() (since we
>> have alrady eliminated one other blemish).
>
> FWIW I consider the automatic close() in dup2() a feature (if only that
> it avoids an additional system call).
Exploiting this "feature" can hide real errors that would be detected
if one does an explicit close(). (See the dup2 man page.)
Cheers,
Michael
On Fri, 2008-10-10 at 15:15 +0200, Michael Kerrisk wrote:
> On Fri, Oct 10, 2008 at 3:02 PM, Bernd Petrovitsch <[email protected]> wrote:
[....]
> >> but to eliminate the blemish in its design in the new dup3() (since we
> >> have alrady eliminated one other blemish).
> >
> > FWIW I consider the automatic close() in dup2() a feature (if only that
> > it avoids an additional system call).
>
> Exploiting this "feature" can hide real errors that would be detected
> if one does an explicit close(). (See the dup2 man page.)
If someone is interested in the return value of the automatic close(),
he/she can call close() anyways and check the the result.
Bernd
--
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and Services
On Thu, Oct 09, 2008 at 01:31:39PM -0700, Ulrich Drepper wrote:
> H. Peter Anvin wrote:
> > The dup2() behavior comes from the logical consequence of dup2()'s
> > "close on reuse"; one would think it would be logical for dup3() to
> > behave the same way.
>
> No. We deliberately decided on this change. Otherwise, what is the
> result of dup3(fd, fd, O_CLOEXEC)? There is no reason to use
> dup2(fd,fd), so why the hell somebody wants to defend this is beyond me.
The reason is: application programmers expect it to behave that way.
The interface is mostly targeted for typical application programmers,
and consistency decreases bugs. In this respect, it would be a good
idea for dup3() to have the same semantics. Doing that might not make
practical sense, but it is secondary to obviousness.
--
Heikki Orsila
[email protected]
http://www.iki.fi/shd