|
|
|
|
|
|
| Author |
Message |
John-Mark Gurney
*nix forums addict
Joined: 16 Jun 2003
Posts: 89
|
 Posted: Mon Feb 27, 2006 6:49 pm Post subject:
Re: changing EINVAL for SIOCSIFCAP to something else
|
|
|
Gleb Smirnoff wrote this message on Mon, Feb 27, 2006 at 11:38 +0300:
| Quote: | I'd like to replace the unpleasant one-for-all error code
EINVAL to something else in this part of code of
src/sys/net/if.c:ifhwioctl()
case SIOCSIFCAP:
error = suser(td);
if (error)
return (error);
if (ifp->if_ioctl == NULL)
return (EOPNOTSUPP);
if (ifr->ifr_reqcap & ~ifp->if_capabilities)
return (EINVAL);
IFF_LOCKGIANT(ifp);
error = (*ifp->if_ioctl)(ifp, cmd, data);
IFF_UNLOCKGIANT(ifp);
if (error == 0)
getmicrotime(&ifp->if_lastchange);
break;
The possible variants are:
#define ENODEV 19 /* Operation not supported by device */
#define ENOTTY 25 /* Inappropriate ioctl for device */
#define ENOPROTOOPT 42 /* Protocol not available */
#define EPROTONOSUPPORT 43 /* Protocol not supported */
I prefer this variant:
if (ifp->if_ioctl == NULL)
return (ENOTTY);
if (ifr->ifr_reqcap & ~ifp->if_capabilities)
return (ENODEV);
Any objections?
|
I'd prefer to keep EINVAL too because the rest of the kernel uses EINVAL
to tell when the parameters passed are incorrect... To me, the real
problem is that the errors that SIOCSIFCAP can return are not documented...
Why not document them so that people will understand what could possibly
be wrong when EINVAL is returned?
This is an example of where more verbose error reporting from the
kernel would be a good idea...
--
John-Mark Gurney Voice: +1 415 225 5579
"All that I will do, has been done, All that I have, has not."
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
 |
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Mon Feb 27, 2006 10:25 am Post subject:
Re: changing EINVAL for SIOCSIFCAP to something else
|
|
|
On Mon, Feb 27, 2006 at 01:20:30PM +0300, Yar Tikhiy wrote:
Y> > Y> > Y> I'm afraid that this is a case when EINVAL is used properly: an
Y> > Y> > Y> argument to ioctl doesn't make sense to a particular device. It's
Y> > Y> > Y> true that EINVAL may be abused in other places though. I wish each
Y> > Y> > Y> EINVAL being returned to the userland were accompanied by log().
Y> > Y> >
Y> > Y> > I don't agree. EINVAL can logically fit to almost any error condition. We
Y> > Y> > should fine error codes fitting better. If "ioctl doesn't make sense to a
Y> > Y> > particular device", then we should say "Operation not supported by device",
Y> > Y> > which is ENODEV.
Y> > Y>
Y> > Y> You see, it isn't ioctl itself that doesn't make sense to the device,
Y> > Y> it's a single argument, ifr_reqcap. That was my point. Of course,
Y> >
Y> > Yes. The ioctl is correct, that's why we do not return ENOTTY. The
Y> > argument is correct, that's why we do not return EINVAL. The argument
Y> > is not applicable to this device, that's why I suggest to use ENODEV.
Y>
Y> This interpretation sounds fair to me. Did you look at other cases
Y> when ENODEV was returned? How consistent were they with this one?
In network code only in if_setlladdr() if the device doesn't have link
level address at all.
In many places throughout the kernel, in most cases close to the description.
AFAIK, EINVAL is a correct choice, when argument is incorrect, for example
its length differs to the expected.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Yar Tikhiy
*nix forums beginner
Joined: 08 Apr 2003
Posts: 36
|
| Posted: Mon Feb 27, 2006 10:20 am Post subject:
Re: changing EINVAL for SIOCSIFCAP to something else
|
|
|
On Mon, Feb 27, 2006 at 01:00:31PM +0300, Gleb Smirnoff wrote:
| Quote: | On Mon, Feb 27, 2006 at 12:44:58PM +0300, Yar Tikhiy wrote:
Y> > On Mon, Feb 27, 2006 at 10:04:28AM +0100, Andre Oppermann wrote:
Y> > A> > I prefer this variant:
Y> > A
Y> > A> > if (ifp->if_ioctl == NULL)
Y> > A> > return (ENOTTY);
Y> > A> > if (ifr->ifr_reqcap & ~ifp->if_capabilities)
Y> > A> > return (ENODEV);
Y> > A
Y> > A> > Any objections?
Y> [...]
Y> > Y> I'm afraid that this is a case when EINVAL is used properly: an
Y> > Y> argument to ioctl doesn't make sense to a particular device. It's
Y> > Y> true that EINVAL may be abused in other places though. I wish each
Y> > Y> EINVAL being returned to the userland were accompanied by log().
Y
Y> > I don't agree. EINVAL can logically fit to almost any error condition. We
Y> > should fine error codes fitting better. If "ioctl doesn't make sense to a
Y> > particular device", then we should say "Operation not supported by device",
Y> > which is ENODEV.
Y
Y> You see, it isn't ioctl itself that doesn't make sense to the device,
Y> it's a single argument, ifr_reqcap. That was my point. Of course,
Yes. The ioctl is correct, that's why we do not return ENOTTY. The
argument is correct, that's why we do not return EINVAL. The argument
is not applicable to this device, that's why I suggest to use ENODEV.
|
This interpretation sounds fair to me. Did you look at other cases
when ENODEV was returned? How consistent were they with this one?
| Quote: | Y> I won't insist on it because the traditional errno is getting very
Y> limited under the present conditions anyway.
|
--
Yar
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Mon Feb 27, 2006 10:00 am Post subject:
Re: changing EINVAL for SIOCSIFCAP to something else
|
|
|
On Mon, Feb 27, 2006 at 12:44:58PM +0300, Yar Tikhiy wrote:
Y> > On Mon, Feb 27, 2006 at 10:04:28AM +0100, Andre Oppermann wrote:
Y> > A> > I prefer this variant:
Y> > A> >
Y> > A> > if (ifp->if_ioctl == NULL)
Y> > A> > return (ENOTTY);
Y> > A> > if (ifr->ifr_reqcap & ~ifp->if_capabilities)
Y> > A> > return (ENODEV);
Y> > A> >
Y> > A> > Any objections?
Y> [...]
Y> > Y> I'm afraid that this is a case when EINVAL is used properly: an
Y> > Y> argument to ioctl doesn't make sense to a particular device. It's
Y> > Y> true that EINVAL may be abused in other places though. I wish each
Y> > Y> EINVAL being returned to the userland were accompanied by log().
Y> >
Y> > I don't agree. EINVAL can logically fit to almost any error condition. We
Y> > should fine error codes fitting better. If "ioctl doesn't make sense to a
Y> > particular device", then we should say "Operation not supported by device",
Y> > which is ENODEV.
Y>
Y> You see, it isn't ioctl itself that doesn't make sense to the device,
Y> it's a single argument, ifr_reqcap. That was my point. Of course,
Yes. The ioctl is correct, that's why we do not return ENOTTY. The
argument is correct, that's why we do not return EINVAL. The argument
is not applicable to this device, that's why I suggest to use ENODEV.
Y> I won't insist on it because the traditional errno is getting very
Y> limited under the present conditions anyway.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Yar Tikhiy
*nix forums beginner
Joined: 08 Apr 2003
Posts: 36
|
| Posted: Mon Feb 27, 2006 9:44 am Post subject:
Re: changing EINVAL for SIOCSIFCAP to something else
|
|
|
On Mon, Feb 27, 2006 at 12:34:31PM +0300, Gleb Smirnoff wrote:
| Quote: | Andre, Yar,
On Mon, Feb 27, 2006 at 10:04:28AM +0100, Andre Oppermann wrote:
A> > I prefer this variant:
A
A> > if (ifp->if_ioctl == NULL)
A> > return (ENOTTY);
A> > if (ifr->ifr_reqcap & ~ifp->if_capabilities)
A> > return (ENODEV);
A
A> > Any objections?
[...]
Y> I'm afraid that this is a case when EINVAL is used properly: an
Y> argument to ioctl doesn't make sense to a particular device. It's
Y> true that EINVAL may be abused in other places though. I wish each
Y> EINVAL being returned to the userland were accompanied by log().
I don't agree. EINVAL can logically fit to almost any error condition. We
should fine error codes fitting better. If "ioctl doesn't make sense to a
particular device", then we should say "Operation not supported by device",
which is ENODEV.
|
You see, it isn't ioctl itself that doesn't make sense to the device,
it's a single argument, ifr_reqcap. That was my point. Of course,
I won't insist on it because the traditional errno is getting very
limited under the present conditions anyway.
--
Yar
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Mon Feb 27, 2006 9:34 am Post subject:
Re: changing EINVAL for SIOCSIFCAP to something else
|
|
|
Andre, Yar,
On Mon, Feb 27, 2006 at 10:04:28AM +0100, Andre Oppermann wrote:
A> > I prefer this variant:
A> >
A> > if (ifp->if_ioctl == NULL)
A> > return (ENOTTY);
A> > if (ifr->ifr_reqcap & ~ifp->if_capabilities)
A> > return (ENODEV);
A> >
A> > Any objections?
A>
A> I don't think ENOTTY is appropriate here even though the comment to this
A> error code would fit. But the define still says no TTY which is totally
A> unrelated and confusing.
It contains a confusing word "tty", but it means "Inappropriate ioctl for device".
This error code is used in many places throughout the kernel. We already have
some ENOTTY returns in src/sys/net.
Y> I'm afraid that this is a case when EINVAL is used properly: an
Y> argument to ioctl doesn't make sense to a particular device. It's
Y> true that EINVAL may be abused in other places though. I wish each
Y> EINVAL being returned to the userland were accompanied by log().
I don't agree. EINVAL can logically fit to almost any error condition. We
should fine error codes fitting better. If "ioctl doesn't make sense to a
particular device", then we should say "Operation not supported by device",
which is ENODEV.
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Yar Tikhiy
*nix forums beginner
Joined: 08 Apr 2003
Posts: 36
|
| Posted: Mon Feb 27, 2006 9:14 am Post subject:
Re: changing EINVAL for SIOCSIFCAP to something else
|
|
|
On Mon, Feb 27, 2006 at 11:38:15AM +0300, Gleb Smirnoff wrote:
| Quote: | Colleagues,
I'd like to replace the unpleasant one-for-all error code
EINVAL to something else in this part of code of
src/sys/net/if.c:ifhwioctl()
case SIOCSIFCAP:
error = suser(td);
if (error)
return (error);
if (ifp->if_ioctl == NULL)
return (EOPNOTSUPP);
if (ifr->ifr_reqcap & ~ifp->if_capabilities)
return (EINVAL);
IFF_LOCKGIANT(ifp);
error = (*ifp->if_ioctl)(ifp, cmd, data);
IFF_UNLOCKGIANT(ifp);
if (error == 0)
getmicrotime(&ifp->if_lastchange);
break;
|
I'm afraid that this is a case when EINVAL is used properly: an
argument to ioctl doesn't make sense to a particular device. It's
true that EINVAL may be abused in other places though. I wish each
EINVAL being returned to the userland were accompanied by log().
--
Yar
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Andre Oppermann
*nix forums addict
Joined: 21 Mar 2002
Posts: 55
|
| Posted: Mon Feb 27, 2006 9:04 am Post subject:
Re: changing EINVAL for SIOCSIFCAP to something else
|
|
|
Gleb Smirnoff wrote:
| Quote: |
Colleagues,
I'd like to replace the unpleasant one-for-all error code
EINVAL to something else in this part of code of
src/sys/net/if.c:ifhwioctl()
case SIOCSIFCAP:
error = suser(td);
if (error)
return (error);
if (ifp->if_ioctl == NULL)
return (EOPNOTSUPP);
if (ifr->ifr_reqcap & ~ifp->if_capabilities)
return (EINVAL);
IFF_LOCKGIANT(ifp);
error = (*ifp->if_ioctl)(ifp, cmd, data);
IFF_UNLOCKGIANT(ifp);
if (error == 0)
getmicrotime(&ifp->if_lastchange);
break;
The possible variants are:
#define ENODEV 19 /* Operation not supported by device */
#define ENOTTY 25 /* Inappropriate ioctl for device */
#define ENOPROTOOPT 42 /* Protocol not available */
#define EPROTONOSUPPORT 43 /* Protocol not supported */
I prefer this variant:
if (ifp->if_ioctl == NULL)
return (ENOTTY);
if (ifr->ifr_reqcap & ~ifp->if_capabilities)
return (ENODEV);
Any objections?
|
I don't think ENOTTY is appropriate here even though the comment to this
error code would fit. But the define still says no TTY which is totally
unrelated and confusing.
--
Andre
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Gleb Smirnoff
*nix forums beginner
Joined: 30 Sep 2005
Posts: 27
|
| Posted: Mon Feb 27, 2006 8:38 am Post subject:
changing EINVAL for SIOCSIFCAP to something else
|
|
|
Colleagues,
I'd like to replace the unpleasant one-for-all error code
EINVAL to something else in this part of code of
src/sys/net/if.c:ifhwioctl()
case SIOCSIFCAP:
error = suser(td);
if (error)
return (error);
if (ifp->if_ioctl == NULL)
return (EOPNOTSUPP);
if (ifr->ifr_reqcap & ~ifp->if_capabilities)
return (EINVAL);
IFF_LOCKGIANT(ifp);
error = (*ifp->if_ioctl)(ifp, cmd, data);
IFF_UNLOCKGIANT(ifp);
if (error == 0)
getmicrotime(&ifp->if_lastchange);
break;
The possible variants are:
#define ENODEV 19 /* Operation not supported by device */
#define ENOTTY 25 /* Inappropriate ioctl for device */
#define ENOPROTOOPT 42 /* Protocol not available */
#define EPROTONOSUPPORT 43 /* Protocol not supported */
I prefer this variant:
if (ifp->if_ioctl == NULL)
return (ENOTTY);
if (ifr->ifr_reqcap & ~ifp->if_capabilities)
return (ENODEV);
Any objections?
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" |
|
| Back to top |
|
|
Google
|
|
| Back to top |
|
|
|
|
The time now is Thu Jan 08, 2009 11:53 am | All times are GMT
|
|
Credit Cards | Learn real Kung Fu | Remortgages | Loans | Send Telegram
|
|
Copyright © 2004-2005 DeniX Solutions SRL
|
|
|
|
Other DeniX Solutions sites:
Unix/Linux blog |
electronics forum |
medicine forum |
science forum |
|
|
Privacy Policy
|
Powered by phpBB © 2001, 2005 phpBB Group
|
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in to check your private messages
Log in
Forum index
»
*nix
»
BSD
»
FreeBSD
»
mail-lists
»
Architecture
