Messages in this thread |  | | Subject | Re: [PATCH] serial167.c: take 2: bugfixes and cleanups | Date | Sat, 26 Aug 2000 10:55:46 +0200 (MEST) | From | (Rogier Wolff) |
| |
Arnaldo Carvalho de Melo wrote: > Hi, > > Please take a look and consider applying.
Consider sending this to Linus then.... ;-)
> - copy_to_user(retinfo,&tmp,sizeof(*retinfo)); > - return 0; > + return copy_to_user(retinfo,&tmp,sizeof(*retinfo)) ? -EFAULT : 0;
Interfaces, interfaces, interfaces.....
I suspect that this is one of the most common uses for copy_to_user.
So wouldn't it make more sense to have copy_to_user (or a variant) return "EFAULT" when it doesn't work?
That would make the code:
return fcopy_to_user(retinfo, &tmp, sizeof (*retinfo));
Looks cleaner to me than the EFAULT case.
When you are "designing" copy_to_user, I can understand the desire to do something "useful" with the return value. If the majority of the callers don't do anything else with the non-zero return than return EFAULT, then that may instead be a good return value for the routine.
I always try to write the code that calls the "support functions" first. As a programmer I'm lazy: I get to write clean code, knowing that the support functions will handle the details. In the end it pays off.
Roger.
(*) I have not checked wether there are ANY users of the old return value. If so, we may need to add a new name for a similar function. If the total lines-of-code is reduced, then I'd say that's a good modification.
-- ** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2137555 ** *-- BitWizard writes Linux device drivers for any device you may have! --* * Common sense is the collection of * ****** prejudices acquired by age eighteen. -- Albert Einstein ******** - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
|  |