* [PATCH] remove TCGETS
@ 2003-08-12 21:48 Matthew Wilcox
2003-08-12 21:59 ` Andreas Dilger
0 siblings, 1 reply; 15+ messages in thread
From: Matthew Wilcox @ 2003-08-12 21:48 UTC (permalink / raw)
To: intermezzo-devel; +Cc: linux-fsdevel
I see no reason to handle TCGETS in presto_ioctl, and even if you do get
it somehow, the action is the same as the default. This happens to break
on PA-RISC and not on i386 due to i386 using an old-style definition of
TCGETS and PA-RISC using one which invovles sizeof().
diff -urpNX dontdiff linus-2.6/fs/intermezzo/dir.c parisc-2.6/fs/intermezzo/dir.c
--- linus-2.6/fs/intermezzo/dir.c Tue Aug 12 13:11:17 2003
+++ parisc-2.6/fs/intermezzo/dir.c Tue Aug 12 13:29:43 2003
@@ -1304,10 +1304,6 @@ int presto_ioctl(struct inode *inode, st
return rc;
}
- case TCGETS:
- EXIT;
- return -EINVAL;
-
default:
EXIT;
return -EINVAL;
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-08-12 21:48 [PATCH] remove TCGETS Matthew Wilcox
@ 2003-08-12 21:59 ` Andreas Dilger
2003-08-13 1:42 ` Peter Braam
2003-08-13 12:12 ` David Woodhouse
0 siblings, 2 replies; 15+ messages in thread
From: Andreas Dilger @ 2003-08-12 21:59 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: intermezzo-devel, linux-fsdevel
On Aug 12, 2003 22:48 +0100, Matthew Wilcox wrote:
> I see no reason to handle TCGETS in presto_ioctl, and even if you do get
> it somehow, the action is the same as the default. This happens to break
> on PA-RISC and not on i386 due to i386 using an old-style definition of
> TCGETS and PA-RISC using one which invovles sizeof().
>
> diff -urpNX dontdiff linus-2.6/fs/intermezzo/dir.c parisc-2.6/fs/intermezzo/dir.c
> --- linus-2.6/fs/intermezzo/dir.c Tue Aug 12 13:11:17 2003
> +++ parisc-2.6/fs/intermezzo/dir.c Tue Aug 12 13:29:43 2003
> @@ -1304,10 +1304,6 @@ int presto_ioctl(struct inode *inode, st
> return rc;
> }
>
> - case TCGETS:
> - EXIT;
> - return -EINVAL;
> -
> default:
> EXIT;
> return -EINVAL;
I can tell you why this was originally in there - because the "default" case
used to print out an error message for unhandled ioctls. Perl used to call
TCGETS all the time on files in lustre, so we put in the no-op case to shut
up the error messages. Now that the error messages are gone from the default
case there is no need to have this there anymore, so removing it is fine.
Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-08-12 21:59 ` Andreas Dilger
@ 2003-08-13 1:42 ` Peter Braam
2003-08-13 12:12 ` David Woodhouse
1 sibling, 0 replies; 15+ messages in thread
From: Peter Braam @ 2003-08-13 1:42 UTC (permalink / raw)
To: Matthew Wilcox, intermezzo-devel, linux-fsdevel
Also we don't use Perl anymore - ditch it.
Thanks Matthew.
- Peter -
On Tue, Aug 12, 2003 at 03:59:56PM -0600, Andreas Dilger wrote:
> On Aug 12, 2003 22:48 +0100, Matthew Wilcox wrote:
> > I see no reason to handle TCGETS in presto_ioctl, and even if you do get
> > it somehow, the action is the same as the default. This happens to break
> > on PA-RISC and not on i386 due to i386 using an old-style definition of
> > TCGETS and PA-RISC using one which invovles sizeof().
> >
> > diff -urpNX dontdiff linus-2.6/fs/intermezzo/dir.c parisc-2.6/fs/intermezzo/dir.c
> > --- linus-2.6/fs/intermezzo/dir.c Tue Aug 12 13:11:17 2003
> > +++ parisc-2.6/fs/intermezzo/dir.c Tue Aug 12 13:29:43 2003
> > @@ -1304,10 +1304,6 @@ int presto_ioctl(struct inode *inode, st
> > return rc;
> > }
> >
> > - case TCGETS:
> > - EXIT;
> > - return -EINVAL;
> > -
> > default:
> > EXIT;
> > return -EINVAL;
>
> I can tell you why this was originally in there - because the "default" case
> used to print out an error message for unhandled ioctls. Perl used to call
> TCGETS all the time on files in lustre, so we put in the no-op case to shut
> up the error messages. Now that the error messages are gone from the default
> case there is no need to have this there anymore, so removing it is fine.
>
> Cheers, Andreas
> --
> Andreas Dilger
> http://sourceforge.net/projects/ext2resize/
> http://www-mddsp.enel.ucalgary.ca/People/adilger/
>
>
>
> -------------------------------------------------------
> This SF.Net email sponsored by: Free pre-built ASP.NET sites including
> Data Reports, E-commerce, Portals, and Forums are available now.
> Download today and enter to win an XBOX or Visual Studio .NET.
> http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
> _______________________________________________
> intermezzo-devel mailing list
> intermezzo-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/intermezzo-devel
- Peter -
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-08-12 21:59 ` Andreas Dilger
2003-08-13 1:42 ` Peter Braam
@ 2003-08-13 12:12 ` David Woodhouse
2003-08-13 15:35 ` Andreas Dilger
1 sibling, 1 reply; 15+ messages in thread
From: David Woodhouse @ 2003-08-13 12:12 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Matthew Wilcox, intermezzo-devel, linux-fsdevel
On Tue, 2003-08-12 at 22:59, Andreas Dilger wrote:
> On Aug 12, 2003 22:48 +0100, Matthew Wilcox wrote:
> > I see no reason to handle TCGETS in presto_ioctl, and even if you do get
> > it somehow, the action is the same as the default. This happens to break
> > on PA-RISC and not on i386 due to i386 using an old-style definition of
> > TCGETS and PA-RISC using one which invovles sizeof().
> >
> > diff -urpNX dontdiff linus-2.6/fs/intermezzo/dir.c parisc-2.6/fs/intermezzo/dir.c
> > --- linus-2.6/fs/intermezzo/dir.c Tue Aug 12 13:11:17 2003
> > +++ parisc-2.6/fs/intermezzo/dir.c Tue Aug 12 13:29:43 2003
> > @@ -1304,10 +1304,6 @@ int presto_ioctl(struct inode *inode, st
> > return rc;
> > }
> >
> > - case TCGETS:
> > - EXIT;
> > - return -EINVAL;
> > -
> > default:
> > EXIT;
> > return -EINVAL;
>
> I can tell you why this was originally in there - because the "default" case
> used to print out an error message for unhandled ioctls. Perl used to call
> TCGETS all the time on files in lustre, so we put in the no-op case to shut
> up the error messages. Now that the error messages are gone from the default
> case there is no need to have this there anymore, so removing it is fine.
Why was it returning -EINVAL instead of -ENOTTY?
--
dwmw2
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-08-13 12:12 ` David Woodhouse
@ 2003-08-13 15:35 ` Andreas Dilger
2003-08-14 1:28 ` Peter Braam
0 siblings, 1 reply; 15+ messages in thread
From: Andreas Dilger @ 2003-08-13 15:35 UTC (permalink / raw)
To: David Woodhouse; +Cc: Matthew Wilcox, intermezzo-devel, linux-fsdevel
On Aug 13, 2003 13:12 +0100, David Woodhouse wrote:
> On Tue, 2003-08-12 at 22:59, Andreas Dilger wrote:
> > On Aug 12, 2003 22:48 +0100, Matthew Wilcox wrote:
> > > I see no reason to handle TCGETS in presto_ioctl, and even if you do get
> > > it somehow, the action is the same as the default. This happens to break
> > > on PA-RISC and not on i386 due to i386 using an old-style definition of
> > > TCGETS and PA-RISC using one which invovles sizeof().
> > >
> > > diff -urpNX dontdiff linus-2.6/fs/intermezzo/dir.c parisc-2.6/fs/intermezzo/dir.c
> > > --- linus-2.6/fs/intermezzo/dir.c Tue Aug 12 13:11:17 2003
> > > +++ parisc-2.6/fs/intermezzo/dir.c Tue Aug 12 13:29:43 2003
> > > @@ -1304,10 +1304,6 @@ int presto_ioctl(struct inode *inode, st
> > > return rc;
> > > }
> > >
> > > - case TCGETS:
> > > - EXIT;
> > > - return -EINVAL;
> > > -
> > > default:
> > > EXIT;
> > > return -EINVAL;
> >
> > I can tell you why this was originally in there - because the "default" case
> > used to print out an error message for unhandled ioctls. Perl used to call
> > TCGETS all the time on files in lustre, so we put in the no-op case to shut
> > up the error messages. Now that the error messages are gone from the default
> > case there is no need to have this there anymore, so removing it is fine.
>
> Why was it returning -EINVAL instead of -ENOTTY?
An oversight. Returning -ENOTTY is what I would do today.
Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-08-13 15:35 ` Andreas Dilger
@ 2003-08-14 1:28 ` Peter Braam
2003-08-14 1:43 ` Matthew Wilcox
0 siblings, 1 reply; 15+ messages in thread
From: Peter Braam @ 2003-08-14 1:28 UTC (permalink / raw)
To: David Woodhouse, Matthew Wilcox, intermezzo-devel, linux-fsdevel
In fact some part of POSIX says that ioctls should return EINVAL
that's why I made that choice. Anyway, the issue is moot.
- Peter -
On Wed, Aug 13, 2003 at 09:35:21AM -0600, Andreas Dilger wrote:
> On Aug 13, 2003 13:12 +0100, David Woodhouse wrote:
> > On Tue, 2003-08-12 at 22:59, Andreas Dilger wrote:
> > > On Aug 12, 2003 22:48 +0100, Matthew Wilcox wrote:
> > > > I see no reason to handle TCGETS in presto_ioctl, and even if you do get
> > > > it somehow, the action is the same as the default. This happens to break
> > > > on PA-RISC and not on i386 due to i386 using an old-style definition of
> > > > TCGETS and PA-RISC using one which invovles sizeof().
> > > >
> > > > diff -urpNX dontdiff linus-2.6/fs/intermezzo/dir.c parisc-2.6/fs/intermezzo/dir.c
> > > > --- linus-2.6/fs/intermezzo/dir.c Tue Aug 12 13:11:17 2003
> > > > +++ parisc-2.6/fs/intermezzo/dir.c Tue Aug 12 13:29:43 2003
> > > > @@ -1304,10 +1304,6 @@ int presto_ioctl(struct inode *inode, st
> > > > return rc;
> > > > }
> > > >
> > > > - case TCGETS:
> > > > - EXIT;
> > > > - return -EINVAL;
> > > > -
> > > > default:
> > > > EXIT;
> > > > return -EINVAL;
> > >
> > > I can tell you why this was originally in there - because the "default" case
> > > used to print out an error message for unhandled ioctls. Perl used to call
> > > TCGETS all the time on files in lustre, so we put in the no-op case to shut
> > > up the error messages. Now that the error messages are gone from the default
> > > case there is no need to have this there anymore, so removing it is fine.
> >
> > Why was it returning -EINVAL instead of -ENOTTY?
>
> An oversight. Returning -ENOTTY is what I would do today.
>
> Cheers, Andreas
> --
> Andreas Dilger
> http://sourceforge.net/projects/ext2resize/
> http://www-mddsp.enel.ucalgary.ca/People/adilger/
>
>
>
> -------------------------------------------------------
> This SF.Net email sponsored by: Free pre-built ASP.NET sites including
> Data Reports, E-commerce, Portals, and Forums are available now.
> Download today and enter to win an XBOX or Visual Studio .NET.
> http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
> _______________________________________________
> intermezzo-devel mailing list
> intermezzo-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/intermezzo-devel
- Peter -
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-08-14 1:28 ` Peter Braam
@ 2003-08-14 1:43 ` Matthew Wilcox
2003-08-14 15:54 ` Bryan Henderson
0 siblings, 1 reply; 15+ messages in thread
From: Matthew Wilcox @ 2003-08-14 1:43 UTC (permalink / raw)
To: Peter Braam
Cc: David Woodhouse, Matthew Wilcox, intermezzo-devel, linux-fsdevel
On Wed, Aug 13, 2003 at 07:28:18PM -0600, Peter Braam wrote:
> In fact some part of POSIX says that ioctls should return EINVAL
> that's why I made that choice. Anyway, the issue is moot.
Actually, it's an interesting point. The relevant bit of SUSv3 says:
If an underlying device driver detects an error, then ioctl() shall fail if:
[EINVAL]
The request or arg argument is not valid for this device.
[EIO]
Some physical I/O error has occurred.
[ENOTTY]
The fildes argument is not associated with a STREAMS device that
accepts control functions.
[ENXIO]
The request and arg arguments are valid for this device driver, but
the service requested cannot be performed on this particular sub-device.
[ENODEV]
The fildes argument refers to a valid STREAMS device, but the
corresponding device driver does not support the ioctl() function.
SUSv2 agrees: http://www.opengroup.org/onlinepubs/007908799/xsh/ioctl.html
Maybe we need to start removing ENOTTY returns from ioctl handlers.
I know, we all find it funny when we're told that something isn't a
typewriter, but ...
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-08-14 1:43 ` Matthew Wilcox
@ 2003-08-14 15:54 ` Bryan Henderson
2003-10-28 15:56 ` David Woodhouse
0 siblings, 1 reply; 15+ messages in thread
From: Bryan Henderson @ 2003-08-14 15:54 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Peter Braam, David Woodhouse, intermezzo-devel, linux-fsdevel,
Matthew Wilcox
>If an underlying device driver detects an error, then ioctl() shall fail
if:
>
>[EINVAL]
> The request or arg argument is not valid for this device.
>...
>[ENOTTY]
> The fildes argument is not associated with a STREAMS device that
> accepts control functions.
>...
>Maybe we need to start removing ENOTTY returns from ioctl handlers.
How do you reach that conclusion from the above? The specification is
ambiguous. When you send a TCGETS to a regular file, both the EINVAL and
ENOTTY cases hold.
The actual spec is even more ambiguous, because it says at the top of the
quoted section that the function described therein doesn't apply to
anything but a STREAMS device, and therefore the implementation can do
anything at all when you send a TCGETS to a regular file and still conform
to that spec.
What's much more important than a spec, though, is convention. There are
probably programs that issue an ioctl to a file descriptor to find out if
it is a terminal or not and expect the conventional ENOTTY for the "no"
case and consider EINVAL an actual error.
--
Bryan Henderson IBM Almaden Research Center
San Jose CA Filesystems
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-08-14 15:54 ` Bryan Henderson
@ 2003-10-28 15:56 ` David Woodhouse
2003-10-28 19:41 ` Bryan Henderson
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: David Woodhouse @ 2003-10-28 15:56 UTC (permalink / raw)
To: Bryan Henderson, torvalds
Cc: Matthew Wilcox, Peter Braam, intermezzo-devel, linux-fsdevel
Linus; executive decision required please:
On Thu, 2003-08-14 at 08:54 -0700, Bryan Henderson wrote:
> <willy quoted SusV2 thusly>:
> >If an underlying device driver detects an error, then ioctl() shall fail
> if:
> >
> >[EINVAL]
> > The request or arg argument is not valid for this device.
> >...
> >[ENOTTY]
> > The fildes argument is not associated with a STREAMS device that
> > accepts control functions.
> >...
> >Maybe we need to start removing ENOTTY returns from ioctl handlers.
>
> How do you reach that conclusion from the above? The specification is
> ambiguous. When you send a TCGETS to a regular file, both the EINVAL and
> ENOTTY cases hold.
>
> The actual spec is even more ambiguous, because it says at the top of the
> quoted section that the function described therein doesn't apply to
> anything but a STREAMS device, and therefore the implementation can do
> anything at all when you send a TCGETS to a regular file and still conform
> to that spec.
>
> What's much more important than a spec, though, is convention. There are
> probably programs that issue an ioctl to a file descriptor to find out if
> it is a terminal or not and expect the conventional ENOTTY for the "no"
> case and consider EINVAL an actual error.
And what's more important than convention is a ruling from the Chief
Penguin.
LTP is bitching at me because my file system returns -EINVAL to all
ioctls instead of -ENOTTY. Do I...
1. Make my file system return -ENOTTY.
2. Make LTP accept -EINVAL instead of -ENOTTY on an invalid ioctl.
3. Make LTP accept _either_ -EINVAL or -ENOTTY in that case.
I favour either #1 or #2 and don't care which. I dislike #3 because we
should be consistent.
cf. http://www.opengroup.org/onlinepubs/007904975/functions/ioctl.html
--
dwmw2
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-10-28 15:56 ` David Woodhouse
@ 2003-10-28 19:41 ` Bryan Henderson
2003-10-28 20:52 ` Andries Brouwer
2003-10-28 21:07 ` Linus Torvalds
2 siblings, 0 replies; 15+ messages in thread
From: Bryan Henderson @ 2003-10-28 19:41 UTC (permalink / raw)
To: David Woodhouse
Cc: Peter Braam, intermezzo-devel, linux-fsdevel, torvalds,
Matthew Wilcox
> 1. Make my file system return -ENOTTY.
> 2. Make LTP accept -EINVAL instead of -ENOTTY on an invalid ioctl.
> 3. Make LTP accept _either_ -EINVAL or -ENOTTY in that case.
I believe there is another choice (but I'm guessing a little at when LTP
does):
4. Make my file system return -ENOTTY to terminal ioctls, and -EINVAL to
everything else.
That's what _I_ ended up doing.
It seems gross because an ioctl handler for a regular file shouldn't have
to know what a terminal ioctl is. But this problem stems from the ancient
view of ioctls that there is a single universal name space of ioctl
commands and every ioctl command means something to everyone, even if it
may be inapplicable in a particular instance. I don't think that view
ever made it beyond terminal ioctls, though.
--
Bryan Henderson IBM Almaden Research Center
San Jose CA Filesystems
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-10-28 15:56 ` David Woodhouse
2003-10-28 19:41 ` Bryan Henderson
@ 2003-10-28 20:52 ` Andries Brouwer
2003-10-28 21:07 ` Linus Torvalds
2 siblings, 0 replies; 15+ messages in thread
From: Andries Brouwer @ 2003-10-28 20:52 UTC (permalink / raw)
To: David Woodhouse
Cc: Bryan Henderson, torvalds, Matthew Wilcox, Peter Braam,
intermezzo-devel, linux-fsdevel
On Tue, Oct 28, 2003 at 03:56:37PM +0000, David Woodhouse wrote:
> LTP is bitching at me because my file system returns -EINVAL to all
> ioctls instead of -ENOTTY. Do I...
>
> 1. Make my file system return -ENOTTY.
> 2. Make LTP accept -EINVAL instead of -ENOTTY on an invalid ioctl.
> 3. Make LTP accept _either_ -EINVAL or -ENOTTY in that case.
Short answer:
#3.
In the call ioctl(fd, SOMEIOCTL, arg) one should return ENOTTY
is fd is wrong, EINVAL if SOMEIOCTL or arg is wrong.
Long answer:
> > >[ENOTTY]
> > > The fildes argument is not associated with a STREAMS device that
> > > accepts control functions.
But you see that POSIX or SUSv* do not specify ioctl at all, except
insofar as it applies to STREAMS devices:
For non-STREAMS devices, the functions performed by this call are unspecified.
So, if you want to decide what error return is appropriate in the
non-STREAMS case, neither POSIX nor SUSv* will help.
So, there are two sources of inspiration. Actual usage in BSD and
similar operating systems, and the POSIX definitions of the error numbers.
The latter are easy to quote:
[ENOTTY]
Inappropriate I/O control operation. A control function has been attempted
for a file or special file for which the operation is inappropriate.
[EINVAL]
Invalid argument. Some invalid argument was supplied; for example, specifying
an undefined signal in a signal() function or a kill() function.
Since both SOMEIOCTL and arg are arguments to the call ioctl(fd, SOMEIOCTL, arg),
it may not be forbidden to return EINVAL.
The POSIX spec mentions ENOTTY in the pages for ioctl, isatty, sockatmark,
tcdrain, tcflow, tcflush, tcgetattr, tcgetpgrp, tcgetsid, tcsendbreak,
tcsetattr, tcsetpgrp, ttyname. Typically ENOTTY points out that the fd parameter
is not a terminal, is not a controlling terminal, is not a socket.
An interesting text fragment is seen in the Rationale:
[EFTYPE] This error code was proposed in earlier proposals as
``Inappropriate operation for file type'', meaning that the operation
requested is not appropriate for the file specified in the function call.
This code was proposed, although the same idea was covered by [ENOTTY],
because the connotations of the name would be misleading. It was pointed out
that the fcntl( ) function uses the error code [EINVAL] for this notion,
and hence all instances of [EFTYPE] were changed to this code.
(Namely, for fcntl( ) the [EINVAL] description says: "... or fd refers to a file
that does not support locking.")
My conclusion is that failing historical custom that would help choosing
between EINVAL and ENOTTY, we should choose EINVAL in all cases where
the decision to return an error was not based on the properties of fd.
Andries
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-10-28 15:56 ` David Woodhouse
2003-10-28 19:41 ` Bryan Henderson
2003-10-28 20:52 ` Andries Brouwer
@ 2003-10-28 21:07 ` Linus Torvalds
2003-10-28 23:51 ` Andrew Sharp
2003-10-29 0:46 ` David Woodhouse
2 siblings, 2 replies; 15+ messages in thread
From: Linus Torvalds @ 2003-10-28 21:07 UTC (permalink / raw)
To: David Woodhouse
Cc: Bryan Henderson, Matthew Wilcox, Peter Braam, intermezzo-devel,
linux-fsdevel
On Tue, 28 Oct 2003, David Woodhouse wrote:
>
> Linus; executive decision required please:
I don't know you you _require_ an executive decision, but the simple fact
is that the regular Linux ioctl() handler always returns ENOTTY if it
doesn't match a ioctl number. See fs/ioctl.c.
In short, the way I think this should be handled is:
- if you don't recognize the ioctl, you should return ENOTTY
- if you recognize the ioctl, but some parameter to the ioctl is wrong,
you should return EINVAL.
This is consistent with file_ioctl(), and also consistent with traditional
uses of ENOTTY. It also just happens to make LTP pass, but I will leave to
you to make up your own mind on whether that is because LTP is a good
test, or whether it's just a small unimportant detail.
Linus
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-10-28 21:07 ` Linus Torvalds
@ 2003-10-28 23:51 ` Andrew Sharp
2003-10-29 1:54 ` Linus Torvalds
2003-10-29 0:46 ` David Woodhouse
1 sibling, 1 reply; 15+ messages in thread
From: Andrew Sharp @ 2003-10-28 23:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Woodhouse, Bryan Henderson, Matthew Wilcox, Peter Braam,
intermezzo-devel, linux-fsdevel
On Tue, Oct 28, 2003 at 01:07:53PM -0800, Linus Torvalds wrote:
>
> On Tue, 28 Oct 2003, David Woodhouse wrote:
> >
> > Linus; executive decision required please:
>
> I don't know you you _require_ an executive decision, but the simple fact
> is that the regular Linux ioctl() handler always returns ENOTTY if it
> doesn't match a ioctl number. See fs/ioctl.c.
>
> In short, the way I think this should be handled is:
> - if you don't recognize the ioctl, you should return ENOTTY
> - if you recognize the ioctl, but some parameter to the ioctl is wrong,
> you should return EINVAL.
>
> This is consistent with file_ioctl(), and also consistent with traditional
> uses of ENOTTY. It also just happens to make LTP pass, but I will leave to
> you to make up your own mind on whether that is because LTP is a good
> test, or whether it's just a small unimportant detail.
Can't we add the range of tty ioctls to file_ioctl so that it can return
ENOTTY so each and every file system driver doesn't have to?
a
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-10-28 21:07 ` Linus Torvalds
2003-10-28 23:51 ` Andrew Sharp
@ 2003-10-29 0:46 ` David Woodhouse
1 sibling, 0 replies; 15+ messages in thread
From: David Woodhouse @ 2003-10-29 0:46 UTC (permalink / raw)
To: Linus Torvalds
Cc: Bryan Henderson, Matthew Wilcox, Peter Braam, intermezzo-devel,
linux-fsdevel
On Tue, 2003-10-28 at 13:07 -0800, Linus Torvalds wrote:
> I don't know you you _require_ an executive decision, but the simple fact
> is that the regular Linux ioctl() handler always returns ENOTTY if it
> doesn't match a ioctl number. See fs/ioctl.c.
I don't require an executive decision to fix my own code -- the patch
went into my CVS tree this morning and is attached. But if I'm going to
hunt down bogus -EINVAL returns and convert them to -ENOTTY, an
executive decision makes my life easier because it stops people bitching
at me for it :)
> In short, the way I think this should be handled is:
> - if you don't recognize the ioctl, you should return ENOTTY
> - if you recognize the ioctl, but some parameter to the ioctl is wrong,
> you should return EINVAL.
Absolutely.
> This is consistent with file_ioctl(), and also consistent with traditional
> uses of ENOTTY. It also just happens to make LTP pass, but I will leave to
> you to make up your own mind on whether that is because LTP is a good
> test, or whether it's just a small unimportant detail.
Life is full of small unimportant details. Being consistent about them
is a quality of implementation issue.
Index: fs/jffs2/ioctl.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/ioctl.c,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -p -r1.7 -r1.8
--- fs/jffs2/ioctl.c 4 Oct 2003 08:33:06 -0000 1.7
+++ fs/jffs2/ioctl.c 28 Oct 2003 16:16:28 -0000 1.8
@@ -1,13 +1,13 @@
/*
* JFFS2 -- Journalling Flash File System, Version 2.
*
- * Copyright (C) 2001 Red Hat, Inc.
+ * Copyright (C) 2001-2003 Red Hat, Inc.
*
* Created by David Woodhouse <dwmw2@redhat.com>
*
* For licensing information, see the file 'LICENCE' in this directory.
*
- * $Id: ioctl.c,v 1.7 2003/10/04 08:33:06 dwmw2 Exp $
+ * $Id: ioctl.c,v 1.8 2003/10/28 16:16:28 dwmw2 Exp $
*
*/
@@ -18,6 +18,6 @@ int jffs2_ioctl(struct inode *inode, str
{
/* Later, this will provide for lsattr.jffs2 and chattr.jffs2, which
will include compression support etc. */
- return -EINVAL;
+ return -ENOTTY;
}
--
dwmw2
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] remove TCGETS
2003-10-28 23:51 ` Andrew Sharp
@ 2003-10-29 1:54 ` Linus Torvalds
0 siblings, 0 replies; 15+ messages in thread
From: Linus Torvalds @ 2003-10-29 1:54 UTC (permalink / raw)
To: Andrew Sharp
Cc: David Woodhouse, Bryan Henderson, Matthew Wilcox, Peter Braam,
intermezzo-devel, linux-fsdevel
On Tue, 28 Oct 2003, Andrew Sharp wrote:
>
> Can't we add the range of tty ioctls to file_ioctl so that it can return
> ENOTTY so each and every file system driver doesn't have to?
Sure. But the filesystem has to return some error _anyway_ for the other
ioctl's it doesn't understand. So what errno would you suggest?
ENOTTY is the one that is what a regular file returns to _all_ ioctl's it
doesn't understand. That's really my point here. ENOTTY is always the
right thing to return - and it has nothing to do with whether the ioctl
number was for a TTY-specific ioctl or not.
Yeah, it's not a very well-named error, but if you actually look at what
glibc prints out, it says
ENOTTY: Inappropriate ioctl for device
and you can test with this extremely stupid program:
test.c:
#include <fcntl.h>
#include <unistd.h>
#include <stdlib.h>
int main(int argc, char **argv)
{
int i;
int fd = open("test.c", O_RDONLY);
srandom(time(NULL));
for (i = 0; i < 100; i++) {
ioctl(fd, random(), 0);
perror("ioctl");
}
}
and verify for youself that it returns ENOTTY regardless of whether the
ioctl number was a TTY-specific one or not.
So to re-iterate: ENOTTY is what everybody _always_ should return if they
don't recognize a number. No "number ranges" involved.
Linus
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2003-10-29 1:56 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-08-12 21:48 [PATCH] remove TCGETS Matthew Wilcox
2003-08-12 21:59 ` Andreas Dilger
2003-08-13 1:42 ` Peter Braam
2003-08-13 12:12 ` David Woodhouse
2003-08-13 15:35 ` Andreas Dilger
2003-08-14 1:28 ` Peter Braam
2003-08-14 1:43 ` Matthew Wilcox
2003-08-14 15:54 ` Bryan Henderson
2003-10-28 15:56 ` David Woodhouse
2003-10-28 19:41 ` Bryan Henderson
2003-10-28 20:52 ` Andries Brouwer
2003-10-28 21:07 ` Linus Torvalds
2003-10-28 23:51 ` Andrew Sharp
2003-10-29 1:54 ` Linus Torvalds
2003-10-29 0:46 ` David Woodhouse
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.