Re: [PATCH] serial167.c: take 2: bugfixes and cleanups

From: Rogier Wolff (R.E.Wolff@BitWizard.nl)
Date: Sat Aug 26 2000 - 03:55:46 EST


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/



This archive was generated by hypermail 2b29 : Thu Aug 31 2000 - 21:00:17 EST