Jan-Hinrich Fessel <oskar@unna.ping.de> writes:
>         /*      
>          * ifr->ifr_data is supposed to point to a struct spppreq.
>          * Check the cmd word first before attempting to fetch all the
>          * data.
>          */
>         if ((subcmd = fuword(ifr->ifr_data)) == -1)
>                 return EFAULT;
> 
>         if (copyin((caddr_t)ifr->ifr_data, &spr, sizeof spr) != 0)
>                 return EFAULT;

(1) copyin does what you want, and probably should be used.

(2) perhaps i'm missing something, but i can see absolutely no reason
why you'd want to do the fuword() followed immediately by the
copyin().  The former is redundant, and wasteful (of both kernel
space, and execution time 8-).

if you're worried about efficiency, this is the wrong kind of
efficiency to worry about, and smacks of unnecessary optimization.

if you're worried about a broken copyin(), beat on the people with the
broken copyin() to fix their copyin().


fuword is fatally flawed in a couple of ways.

(1) "what's a 'word'"?  It's not a well-defined type, though it could
be defined in this instance to mean 'int' or 'long' or whatever.
however, if we're going to generally support the fu* and su* APIs,
they really should be complete, i.e. support char/short/int/long, as
well as fixed-size types.

(2) how do you tell the difference between an error and having read
the value with the bit pattern corresponding to -1?  (you don't.)


So, in a nutshell: fu* and su* as they exist now are fundamentally
broken and for most purposes should not be used, and, as far as I can
tell, for your purpose, it's only wasteful to try and use them anyway!



-- 
Chris Demetriou - cgd@netbsd.org - http://www.netbsd.org/People/Pages/cgd.html
Disclaimer: Not speaking for NetBSD, just expressing my own opinion.