linux-mips
[Top] [All Lists]

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

To: Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH V2 05/10] MIPS: lantiq: add watchdog support
From: Sergei Shtylyov <sshtylyov@mvista.com>
Date: Wed, 02 Mar 2011 20:34:52 +0300
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: <20110302142933.GA18221@linux-mips.org>
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> <20110302142933.GA18221@linux-mips.org>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Thunderbird 2.0.0.21 (X11/20090320)
Hello.

Ralf Baechle 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.

But then the code above would be wrong. Actually, it returns the number of bytes that could NOT be copied as I now see.

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.

   Indeed.

  Ralf

WBR, Sergei

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