* [patch] watchdog: ts72xx_wdt: cleanup return codes in ioctl
@ 2013-08-23 9:38 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2013-08-23 9:38 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-watchdog, kernel-janitors
There seems to be some confusion here which functions return positive
numbers and which return negative error codes.
copy_to_user() returns the number of bytes remaining to be copied but we
want to return -EFAULT.
The rest is just clean up. get_user() actually returns zero on success
and -EFAULT on error so we can preserve the error code. The
timeout_to_regval() function returns -EINVAL on failure, but we can
propogate that back instead of hardcoding -EINVAL ourselves.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/watchdog/ts72xx_wdt.c b/drivers/watchdog/ts72xx_wdt.c
index 4da59b4..9125b0f 100644
--- a/drivers/watchdog/ts72xx_wdt.c
+++ b/drivers/watchdog/ts72xx_wdt.c
@@ -305,7 +305,8 @@ static long ts72xx_wdt_ioctl(struct file *file, unsigned int cmd,
switch (cmd) {
case WDIOC_GETSUPPORT:
- error = copy_to_user(argp, &winfo, sizeof(winfo));
+ if (copy_to_user(argp, &winfo, sizeof(winfo)))
+ error = -EFAULT;
break;
case WDIOC_GETSTATUS:
@@ -319,10 +320,9 @@ static long ts72xx_wdt_ioctl(struct file *file, unsigned int cmd,
case WDIOC_SETOPTIONS: {
int options;
- if (get_user(options, p)) {
- error = -EFAULT;
+ error = get_user(options, p);
+ if (error)
break;
- }
error = -EINVAL;
@@ -340,30 +340,26 @@ static long ts72xx_wdt_ioctl(struct file *file, unsigned int cmd,
case WDIOC_SETTIMEOUT: {
int new_timeout;
+ int regval;
- if (get_user(new_timeout, p)) {
- error = -EFAULT;
- } else {
- int regval;
-
- regval = timeout_to_regval(new_timeout);
- if (regval < 0) {
- error = -EINVAL;
- } else {
- ts72xx_wdt_stop(wdt);
- wdt->regval = regval;
- ts72xx_wdt_start(wdt);
- }
- }
+ error = get_user(new_timeout, p);
if (error)
break;
+ regval = timeout_to_regval(new_timeout);
+ if (regval < 0) {
+ error = regval;
+ break;
+ }
+ ts72xx_wdt_stop(wdt);
+ wdt->regval = regval;
+ ts72xx_wdt_start(wdt);
+
/*FALLTHROUGH*/
}
case WDIOC_GETTIMEOUT:
- if (put_user(regval_to_timeout(wdt->regval), p))
- error = -EFAULT;
+ error = put_user(regval_to_timeout(wdt->regval), p);
break;
default:
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [patch] watchdog: ts72xx_wdt: cleanup return codes in ioctl
@ 2013-08-23 9:38 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2013-08-23 9:38 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-watchdog, kernel-janitors
There seems to be some confusion here which functions return positive
numbers and which return negative error codes.
copy_to_user() returns the number of bytes remaining to be copied but we
want to return -EFAULT.
The rest is just clean up. get_user() actually returns zero on success
and -EFAULT on error so we can preserve the error code. The
timeout_to_regval() function returns -EINVAL on failure, but we can
propogate that back instead of hardcoding -EINVAL ourselves.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/watchdog/ts72xx_wdt.c b/drivers/watchdog/ts72xx_wdt.c
index 4da59b4..9125b0f 100644
--- a/drivers/watchdog/ts72xx_wdt.c
+++ b/drivers/watchdog/ts72xx_wdt.c
@@ -305,7 +305,8 @@ static long ts72xx_wdt_ioctl(struct file *file, unsigned int cmd,
switch (cmd) {
case WDIOC_GETSUPPORT:
- error = copy_to_user(argp, &winfo, sizeof(winfo));
+ if (copy_to_user(argp, &winfo, sizeof(winfo)))
+ error = -EFAULT;
break;
case WDIOC_GETSTATUS:
@@ -319,10 +320,9 @@ static long ts72xx_wdt_ioctl(struct file *file, unsigned int cmd,
case WDIOC_SETOPTIONS: {
int options;
- if (get_user(options, p)) {
- error = -EFAULT;
+ error = get_user(options, p);
+ if (error)
break;
- }
error = -EINVAL;
@@ -340,30 +340,26 @@ static long ts72xx_wdt_ioctl(struct file *file, unsigned int cmd,
case WDIOC_SETTIMEOUT: {
int new_timeout;
+ int regval;
- if (get_user(new_timeout, p)) {
- error = -EFAULT;
- } else {
- int regval;
-
- regval = timeout_to_regval(new_timeout);
- if (regval < 0) {
- error = -EINVAL;
- } else {
- ts72xx_wdt_stop(wdt);
- wdt->regval = regval;
- ts72xx_wdt_start(wdt);
- }
- }
+ error = get_user(new_timeout, p);
if (error)
break;
+ regval = timeout_to_regval(new_timeout);
+ if (regval < 0) {
+ error = regval;
+ break;
+ }
+ ts72xx_wdt_stop(wdt);
+ wdt->regval = regval;
+ ts72xx_wdt_start(wdt);
+
/*FALLTHROUGH*/
}
case WDIOC_GETTIMEOUT:
- if (put_user(regval_to_timeout(wdt->regval), p))
- error = -EFAULT;
+ error = put_user(regval_to_timeout(wdt->regval), p);
break;
default:
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: watchdog: ts72xx_wdt: cleanup return codes in ioctl
2013-08-23 9:38 ` Dan Carpenter
@ 2013-10-01 15:39 ` Guenter Roeck
-1 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2013-10-01 15:39 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Wim Van Sebroeck, linux-watchdog, kernel-janitors
On Fri, Aug 23, 2013 at 05:38:32PM -0000, Dan Carpenter wrote:
> There seems to be some confusion here which functions return positive
> numbers and which return negative error codes.
>
> copy_to_user() returns the number of bytes remaining to be copied but we
> want to return -EFAULT.
>
> The rest is just clean up. get_user() actually returns zero on success
> and -EFAULT on error so we can preserve the error code. The
> timeout_to_regval() function returns -EINVAL on failure, but we can
> propogate that back instead of hardcoding -EINVAL ourselves.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: watchdog: ts72xx_wdt: cleanup return codes in ioctl
@ 2013-10-01 15:39 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2013-10-01 15:39 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Wim Van Sebroeck, linux-watchdog, kernel-janitors
On Fri, Aug 23, 2013 at 05:38:32PM -0000, Dan Carpenter wrote:
> There seems to be some confusion here which functions return positive
> numbers and which return negative error codes.
>
> copy_to_user() returns the number of bytes remaining to be copied but we
> want to return -EFAULT.
>
> The rest is just clean up. get_user() actually returns zero on success
> and -EFAULT on error so we can preserve the error code. The
> timeout_to_regval() function returns -EINVAL on failure, but we can
> propogate that back instead of hardcoding -EINVAL ourselves.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] watchdog: ts72xx_wdt: cleanup return codes in ioctl
2013-08-23 9:38 ` Dan Carpenter
@ 2013-10-29 7:44 ` Wim Van Sebroeck
-1 siblings, 0 replies; 6+ messages in thread
From: Wim Van Sebroeck @ 2013-10-29 7:44 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-watchdog, kernel-janitors
Hi Dan,
> There seems to be some confusion here which functions return positive
> numbers and which return negative error codes.
>
> copy_to_user() returns the number of bytes remaining to be copied but we
> want to return -EFAULT.
>
> The rest is just clean up. get_user() actually returns zero on success
> and -EFAULT on error so we can preserve the error code. The
> timeout_to_regval() function returns -EINVAL on failure, but we can
> propogate that back instead of hardcoding -EINVAL ourselves.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
This patch has been added to linux-watchdog-next.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] watchdog: ts72xx_wdt: cleanup return codes in ioctl
@ 2013-10-29 7:44 ` Wim Van Sebroeck
0 siblings, 0 replies; 6+ messages in thread
From: Wim Van Sebroeck @ 2013-10-29 7:44 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-watchdog, kernel-janitors
Hi Dan,
> There seems to be some confusion here which functions return positive
> numbers and which return negative error codes.
>
> copy_to_user() returns the number of bytes remaining to be copied but we
> want to return -EFAULT.
>
> The rest is just clean up. get_user() actually returns zero on success
> and -EFAULT on error so we can preserve the error code. The
> timeout_to_regval() function returns -EINVAL on failure, but we can
> propogate that back instead of hardcoding -EINVAL ourselves.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
This patch has been added to linux-watchdog-next.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-29 7:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-23 9:38 [patch] watchdog: ts72xx_wdt: cleanup return codes in ioctl Dan Carpenter
2013-08-23 9:38 ` Dan Carpenter
2013-10-01 15:39 ` Guenter Roeck
2013-10-01 15:39 ` Guenter Roeck
2013-10-29 7:44 ` [patch] " Wim Van Sebroeck
2013-10-29 7:44 ` Wim Van Sebroeck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.