changing EINVAL for SIOCSIFCAP to something else

[John-Mark Gurney]

Gleb Smirnoff wrote this message on Mon, Feb 27, 2006 at 11:38 +0300:
>   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."