Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sat, 7 Jun 2003 13:05:29 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [PATCH] fix check warnings in drivers/macintosh |
| |
On Sat, 7 Jun 2003, Paul Mackerras wrote: > > This patch removes the warnings that the `check' program came up with > in drivers/macintosh. This involves adding __user in various places > and fixing some non-ANSI function definitions for functions that take > no arguments.
Thanks, but please avoid doing things like this:
> diff -urN linux-2.5/drivers/macintosh/macserial.c pmac-2.5/drivers/macintosh/macserial.c > --- linux-2.5/drivers/macintosh/macserial.c 2003-06-07 08:57:42.000000000 +1000 > +++ pmac-2.5/drivers/macintosh/macserial.c 2003-06-07 14:35:34.000000000 +1000 > @@ -1496,7 +1496,7 @@ > if (c <= 0) > break; > > - c -= copy_from_user(tmp_buf, buf, c); > + c -= copy_from_user(tmp_buf, (void __user *) buf, c);
The problem here is that "buf" has the wrong type, and you're hiding it with a cast.
That's pointless - yes, it avoids a warning from "check", but the thing is, that warning was _correct_, and you just hid the problem.
If "buf" really is a user pointer, then it should have been marked that way in the first place.
I understand why you did it the way you did: the tty functions are badly designed, and the same function is used for both user and kernel pointers.
And that's something that check does and _should_ warn about. The function definition is crap, and I'd rather have the warning there and hope some tty layer person comes along and makes the user/kernel case into separate functions (they don't share that much code anyway, since most of the function is
if (from_user) { user case } else { kernel case }
anyway.
I hate these kinds of "flag" arguments anyway. If a function does two different things, it should be two different functions. Yeah, I know, the "gfp_mask" thing for the memory allocators are the same way, and yeah, that was a design mistake too.
Linus
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |