linux-mips
[Top] [All Lists]

Re: [PATCH V2 05/10] MIPS: lantiq: add watchdog support

To: Sergei Shtylyov <sshtylyov@mvista.com>
Subject: Re: [PATCH V2 05/10] MIPS: lantiq: add watchdog support
From: Ralf Baechle <ralf@linux-mips.org>
Date: Wed, 2 Mar 2011 15:29:33 +0100
Cc: John Crispin <blogic@openwrt.org>, Ralph Hempel <ralph.hempel@lantiq.com>, Wim Van Sebroeck <wim@iguana.be>, linux-mips@linux-mips.org, linux-watchdog@vger.kernel.org
In-reply-to: <4D6E286D.9050100@mvista.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1298996006-15960-1-git-send-email-blogic@openwrt.org> <1298996006-15960-6-git-send-email-blogic@openwrt.org> <4D6E286D.9050100@mvista.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Mar 02, 2011 at 02:22:21PM +0300, Sergei Shtylyov wrote:

> >+    switch (cmd) {
> >+    case WDIOC_GETSUPPORT:
> >+            ret = copy_to_user((struct watchdog_info __user *)arg, &ident,
> >+                            sizeof(ident)) ? -EFAULT : 0;
> 
>    Doesn't copy_to_user() return 0 or -EFAULT?

No and that's a common cause of bugs.  copy_{from,to}_user returns the
number of characters that could be be copied so the conversion to an
error code is needed here.

The function takes a void argument and there is no benefit from casting
to the full struct watchdog_info __user * pointer type other than maybe
clarity to the human reader.

While nitpicking - there should be one space between include and < in
#include <blah.h>.

  Ralf

<Prev in Thread] Current Thread [Next in Thread>