LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Fix some incorrect use of positive error codes.
@ 2010-12-26 5:57 Ralph Loader
2010-12-27 11:27 ` Jiri Kosina
0 siblings, 1 reply; 6+ messages in thread
From: Ralph Loader @ 2010-12-26 5:57 UTC (permalink / raw)
To: linux-kernel, trivial
Fix a few places where positive EFOO error codes were being returned
where negative error codes are required.
Most of these are trivial. Explanations of the other two:
cfrfml.c:cfrfml_receive() was returning -EPROTO in one place and
(+) EPROTO in another. Delete the positive case; the return code has
already been initialised to -EPROTO at that point.
In arch/ppc, cmm.c:cmm_memory_cb() was passing a positive error-code
returned from cmm_mem_going_offline() to notifier_from_errno().
But the latter expects negative error codes. Resolve the inconsistency
by returning the 'notifier' value from cmm_mem_going_offline() in the
first place.
These were found using coccinelle to grep for returning positive EFOO,
and then going through the 300 or so hits to find a dozen or so
that were obviously wrong. It is almost certain that I missed some
problems - some places (notably XFS and a few drivers in staging) use
both positive and negative error codes, making it difficult to know
which is intended where.
Signed-off-by: Ralph Loader <suckfish@ihug.co.nz>
---
arch/powerpc/platforms/pseries/cmm.c | 18 +++++-------------
drivers/staging/ath6kl/os/linux/ioctl.c | 10 +++++-----
drivers/staging/brcm80211/brcmfmac/wl_iw.c | 2 +-
drivers/staging/cxt1e1/linux.c | 2 +-
.../staging/westbridge/astoria/device/cyasdevice.c | 2 +-
fs/cifs/cifsencrypt.c | 2 +-
net/caif/cfrfml.c | 1 -
7 files changed, 14 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
index f480386..3542fdc 100644
--- a/arch/powerpc/platforms/pseries/cmm.c
+++ b/arch/powerpc/platforms/pseries/cmm.c
@@ -526,7 +526,7 @@ static struct notifier_block cmm_mem_isolate_nb = {
* @arg: memory_notify structure with page range to be offlined
*
* Return value:
- * 0 on success
+ * NOTIFY_OK on success
**/
static int cmm_mem_going_offline(void *arg)
{
@@ -581,7 +581,7 @@ static int cmm_mem_going_offline(void *arg)
cmm_dbg("Failed to allocate memory for list "
"management. Memory hotplug "
"failed.\n");
- return ENOMEM;
+ return notifier_from_errno(-ENOMEM);
}
memcpy(npa, pa_curr, PAGE_SIZE);
if (pa_curr == cmm_page_list)
@@ -600,7 +600,7 @@ static int cmm_mem_going_offline(void *arg)
spin_unlock(&cmm_lock);
cmm_dbg("Released %ld pages in the search range.\n", freed);
- return 0;
+ return NOTIFY_OK;
}
/**
@@ -616,14 +616,11 @@ static int cmm_mem_going_offline(void *arg)
static int cmm_memory_cb(struct notifier_block *self,
unsigned long action, void *arg)
{
- int ret = 0;
-
switch (action) {
case MEM_GOING_OFFLINE:
mutex_lock(&hotplug_mutex);
hotplug_occurred = 1;
- ret = cmm_mem_going_offline(arg);
- break;
+ return cmm_mem_going_offline(arg);
case MEM_OFFLINE:
case MEM_CANCEL_OFFLINE:
mutex_unlock(&hotplug_mutex);
@@ -635,12 +632,7 @@ static int cmm_memory_cb(struct notifier_block *self,
break;
}
- if (ret)
- ret = notifier_from_errno(ret);
- else
- ret = NOTIFY_OK;
-
- return ret;
+ return NOTIFY_OK;
}
static struct notifier_block cmm_mem_nb = {
diff --git a/drivers/staging/ath6kl/os/linux/ioctl.c b/drivers/staging/ath6kl/os/linux/ioctl.c
index d5f7ac0..0b10376 100644
--- a/drivers/staging/ath6kl/os/linux/ioctl.c
+++ b/drivers/staging/ath6kl/os/linux/ioctl.c
@@ -440,7 +440,7 @@ ar6000_ioctl_set_rssi_threshold(struct net_device *dev, struct ifreq *rq)
if (ar->rssi_map[j+1].rssi < ar->rssi_map[j].rssi) {
SWAP_THOLD(ar->rssi_map[j+1], ar->rssi_map[j]);
} else if (ar->rssi_map[j+1].rssi == ar->rssi_map[j].rssi) {
- return EFAULT;
+ return -EFAULT;
}
}
}
@@ -449,7 +449,7 @@ ar6000_ioctl_set_rssi_threshold(struct net_device *dev, struct ifreq *rq)
if (ar->rssi_map[j+1].rssi < ar->rssi_map[j].rssi) {
SWAP_THOLD(ar->rssi_map[j+1], ar->rssi_map[j]);
} else if (ar->rssi_map[j+1].rssi == ar->rssi_map[j].rssi) {
- return EFAULT;
+ return -EFAULT;
}
}
}
@@ -2870,7 +2870,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
gpio_output_set_cmd.enable_mask,
gpio_output_set_cmd.disable_mask);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
}
up(&ar->arSem);
@@ -2950,7 +2950,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
gpio_register_cmd.gpioreg_id,
gpio_register_cmd.value);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
/* Wait for acknowledgement from Target */
@@ -3041,7 +3041,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
} else {
ret = ar6000_gpio_intr_ack(dev, gpio_intr_ack_cmd.ack_mask);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
}
up(&ar->arSem);
diff --git a/drivers/staging/brcm80211/brcmfmac/wl_iw.c b/drivers/staging/brcm80211/brcmfmac/wl_iw.c
index 979a494..b68bc54 100644
--- a/drivers/staging/brcm80211/brcmfmac/wl_iw.c
+++ b/drivers/staging/brcm80211/brcmfmac/wl_iw.c
@@ -1369,7 +1369,7 @@ wl_iw_iscan_set_scan(struct net_device *dev,
if (g_scan_specified_ssid) {
WL_TRACE(("%s Specific SCAN already running ignoring BC scan\n",
__func__));
- return EBUSY;
+ return -EBUSY;
}
memset(&ssid, 0, sizeof(ssid));
diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
index c793028..7af4887 100644
--- a/drivers/staging/cxt1e1/linux.c
+++ b/drivers/staging/cxt1e1/linux.c
@@ -548,7 +548,7 @@ do_set_port (struct net_device * ndev, void *data)
return -EINVAL; /* get card info */
if (pp.portnum >= ci->max_port) /* sanity check */
- return ENXIO;
+ return -ENXIO;
memcpy (&ci->port[pp.portnum].p, &pp, sizeof (struct sbecom_port_param));
return mkret (c4_set_port (ci, pp.portnum));
diff --git a/drivers/staging/westbridge/astoria/device/cyasdevice.c b/drivers/staging/westbridge/astoria/device/cyasdevice.c
index c76e383..0889730 100644
--- a/drivers/staging/westbridge/astoria/device/cyasdevice.c
+++ b/drivers/staging/westbridge/astoria/device/cyasdevice.c
@@ -389,7 +389,7 @@ EXPORT_SYMBOL(cyasdevice_gethaltag);
static int __init cyasdevice_init(void)
{
if (cyasdevice_initialize() != 0)
- return ENODEV;
+ return -ENODEV;
return 0;
}
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index f856732..76df22b 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -585,7 +585,7 @@ setup_ntlmv2_rsp(struct cifsSesInfo *ses, const struct nls_table *nls_cp)
ses->auth_key.response = kmalloc(baselen + tilen, GFP_KERNEL);
if (!ses->auth_key.response) {
- rc = ENOMEM;
+ rc = -ENOMEM;
ses->auth_key.len = 0;
cERROR(1, "%s: Can't allocate auth blob", __func__);
goto setup_ntlmv2_rsp_ret;
diff --git a/net/caif/cfrfml.c b/net/caif/cfrfml.c
index e2fb5fa..f5531c6 100644
--- a/net/caif/cfrfml.c
+++ b/net/caif/cfrfml.c
@@ -162,7 +162,6 @@ static int cfrfml_receive(struct cflayer *layr, struct cfpkt *pkt)
tmppkt = NULL;
/* Verify that length is correct */
- err = EPROTO;
if (rfml->pdu_size != cfpkt_getlen(pkt) - RFM_HEAD_SIZE + 1)
goto out;
}
--
1.7.3.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix some incorrect use of positive error codes.
2010-12-26 5:57 [PATCH] Fix some incorrect use of positive error codes Ralph Loader
@ 2010-12-27 11:27 ` Jiri Kosina
2010-12-28 23:01 ` Ralph Loader
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Kosina @ 2010-12-27 11:27 UTC (permalink / raw)
To: Ralph Loader; +Cc: linux-kernel
On Sun, 26 Dec 2010, Ralph Loader wrote:
> Fix a few places where positive EFOO error codes were being returned
> where negative error codes are required.
>
> Most of these are trivial. Explanations of the other two:
>
> cfrfml.c:cfrfml_receive() was returning -EPROTO in one place and
> (+) EPROTO in another. Delete the positive case; the return code has
> already been initialised to -EPROTO at that point.
>
> In arch/ppc, cmm.c:cmm_memory_cb() was passing a positive error-code
> returned from cmm_mem_going_offline() to notifier_from_errno().
> But the latter expects negative error codes. Resolve the inconsistency
> by returning the 'notifier' value from cmm_mem_going_offline() in the
> first place.
>
> These were found using coccinelle to grep for returning positive EFOO,
> and then going through the 300 or so hits to find a dozen or so
> that were obviously wrong. It is almost certain that I missed some
> problems - some places (notably XFS and a few drivers in staging) use
> both positive and negative error codes, making it difficult to know
> which is intended where.
Could you please split the staging bits and submit them separately to
Greg?
Thanks.
>
> Signed-off-by: Ralph Loader <suckfish@ihug.co.nz>
> ---
> arch/powerpc/platforms/pseries/cmm.c | 18 +++++-------------
> drivers/staging/ath6kl/os/linux/ioctl.c | 10 +++++-----
> drivers/staging/brcm80211/brcmfmac/wl_iw.c | 2 +-
> drivers/staging/cxt1e1/linux.c | 2 +-
> .../staging/westbridge/astoria/device/cyasdevice.c | 2 +-
> fs/cifs/cifsencrypt.c | 2 +-
> net/caif/cfrfml.c | 1 -
> 7 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index f480386..3542fdc 100644
> --- a/arch/powerpc/platforms/pseries/cmm.c
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -526,7 +526,7 @@ static struct notifier_block cmm_mem_isolate_nb = {
> * @arg: memory_notify structure with page range to be offlined
> *
> * Return value:
> - * 0 on success
> + * NOTIFY_OK on success
> **/
> static int cmm_mem_going_offline(void *arg)
> {
> @@ -581,7 +581,7 @@ static int cmm_mem_going_offline(void *arg)
> cmm_dbg("Failed to allocate memory for list "
> "management. Memory hotplug "
> "failed.\n");
> - return ENOMEM;
> + return notifier_from_errno(-ENOMEM);
> }
> memcpy(npa, pa_curr, PAGE_SIZE);
> if (pa_curr == cmm_page_list)
> @@ -600,7 +600,7 @@ static int cmm_mem_going_offline(void *arg)
> spin_unlock(&cmm_lock);
> cmm_dbg("Released %ld pages in the search range.\n", freed);
>
> - return 0;
> + return NOTIFY_OK;
> }
>
> /**
> @@ -616,14 +616,11 @@ static int cmm_mem_going_offline(void *arg)
> static int cmm_memory_cb(struct notifier_block *self,
> unsigned long action, void *arg)
> {
> - int ret = 0;
> -
> switch (action) {
> case MEM_GOING_OFFLINE:
> mutex_lock(&hotplug_mutex);
> hotplug_occurred = 1;
> - ret = cmm_mem_going_offline(arg);
> - break;
> + return cmm_mem_going_offline(arg);
> case MEM_OFFLINE:
> case MEM_CANCEL_OFFLINE:
> mutex_unlock(&hotplug_mutex);
> @@ -635,12 +632,7 @@ static int cmm_memory_cb(struct notifier_block *self,
> break;
> }
>
> - if (ret)
> - ret = notifier_from_errno(ret);
> - else
> - ret = NOTIFY_OK;
> -
> - return ret;
> + return NOTIFY_OK;
> }
>
> static struct notifier_block cmm_mem_nb = {
> diff --git a/drivers/staging/ath6kl/os/linux/ioctl.c b/drivers/staging/ath6kl/os/linux/ioctl.c
> index d5f7ac0..0b10376 100644
> --- a/drivers/staging/ath6kl/os/linux/ioctl.c
> +++ b/drivers/staging/ath6kl/os/linux/ioctl.c
> @@ -440,7 +440,7 @@ ar6000_ioctl_set_rssi_threshold(struct net_device *dev, struct ifreq *rq)
> if (ar->rssi_map[j+1].rssi < ar->rssi_map[j].rssi) {
> SWAP_THOLD(ar->rssi_map[j+1], ar->rssi_map[j]);
> } else if (ar->rssi_map[j+1].rssi == ar->rssi_map[j].rssi) {
> - return EFAULT;
> + return -EFAULT;
> }
> }
> }
> @@ -449,7 +449,7 @@ ar6000_ioctl_set_rssi_threshold(struct net_device *dev, struct ifreq *rq)
> if (ar->rssi_map[j+1].rssi < ar->rssi_map[j].rssi) {
> SWAP_THOLD(ar->rssi_map[j+1], ar->rssi_map[j]);
> } else if (ar->rssi_map[j+1].rssi == ar->rssi_map[j].rssi) {
> - return EFAULT;
> + return -EFAULT;
> }
> }
> }
> @@ -2870,7 +2870,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> gpio_output_set_cmd.enable_mask,
> gpio_output_set_cmd.disable_mask);
> if (ret != A_OK) {
> - ret = EIO;
> + ret = -EIO;
> }
> }
> up(&ar->arSem);
> @@ -2950,7 +2950,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> gpio_register_cmd.gpioreg_id,
> gpio_register_cmd.value);
> if (ret != A_OK) {
> - ret = EIO;
> + ret = -EIO;
> }
>
> /* Wait for acknowledgement from Target */
> @@ -3041,7 +3041,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> } else {
> ret = ar6000_gpio_intr_ack(dev, gpio_intr_ack_cmd.ack_mask);
> if (ret != A_OK) {
> - ret = EIO;
> + ret = -EIO;
> }
> }
> up(&ar->arSem);
> diff --git a/drivers/staging/brcm80211/brcmfmac/wl_iw.c b/drivers/staging/brcm80211/brcmfmac/wl_iw.c
> index 979a494..b68bc54 100644
> --- a/drivers/staging/brcm80211/brcmfmac/wl_iw.c
> +++ b/drivers/staging/brcm80211/brcmfmac/wl_iw.c
> @@ -1369,7 +1369,7 @@ wl_iw_iscan_set_scan(struct net_device *dev,
> if (g_scan_specified_ssid) {
> WL_TRACE(("%s Specific SCAN already running ignoring BC scan\n",
> __func__));
> - return EBUSY;
> + return -EBUSY;
> }
>
> memset(&ssid, 0, sizeof(ssid));
> diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
> index c793028..7af4887 100644
> --- a/drivers/staging/cxt1e1/linux.c
> +++ b/drivers/staging/cxt1e1/linux.c
> @@ -548,7 +548,7 @@ do_set_port (struct net_device * ndev, void *data)
> return -EINVAL; /* get card info */
>
> if (pp.portnum >= ci->max_port) /* sanity check */
> - return ENXIO;
> + return -ENXIO;
>
> memcpy (&ci->port[pp.portnum].p, &pp, sizeof (struct sbecom_port_param));
> return mkret (c4_set_port (ci, pp.portnum));
> diff --git a/drivers/staging/westbridge/astoria/device/cyasdevice.c b/drivers/staging/westbridge/astoria/device/cyasdevice.c
> index c76e383..0889730 100644
> --- a/drivers/staging/westbridge/astoria/device/cyasdevice.c
> +++ b/drivers/staging/westbridge/astoria/device/cyasdevice.c
> @@ -389,7 +389,7 @@ EXPORT_SYMBOL(cyasdevice_gethaltag);
> static int __init cyasdevice_init(void)
> {
> if (cyasdevice_initialize() != 0)
> - return ENODEV;
> + return -ENODEV;
>
> return 0;
> }
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index f856732..76df22b 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -585,7 +585,7 @@ setup_ntlmv2_rsp(struct cifsSesInfo *ses, const struct nls_table *nls_cp)
>
> ses->auth_key.response = kmalloc(baselen + tilen, GFP_KERNEL);
> if (!ses->auth_key.response) {
> - rc = ENOMEM;
> + rc = -ENOMEM;
> ses->auth_key.len = 0;
> cERROR(1, "%s: Can't allocate auth blob", __func__);
> goto setup_ntlmv2_rsp_ret;
> diff --git a/net/caif/cfrfml.c b/net/caif/cfrfml.c
> index e2fb5fa..f5531c6 100644
> --- a/net/caif/cfrfml.c
> +++ b/net/caif/cfrfml.c
> @@ -162,7 +162,6 @@ static int cfrfml_receive(struct cflayer *layr, struct cfpkt *pkt)
> tmppkt = NULL;
>
> /* Verify that length is correct */
> - err = EPROTO;
> if (rfml->pdu_size != cfpkt_getlen(pkt) - RFM_HEAD_SIZE + 1)
> goto out;
> }
> --
> 1.7.3.4
>
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix some incorrect use of positive error codes.
2010-12-27 11:27 ` Jiri Kosina
@ 2010-12-28 23:01 ` Ralph Loader
2010-12-28 23:04 ` Ralph Loader
2011-01-21 0:05 ` Greg KH
0 siblings, 2 replies; 6+ messages in thread
From: Ralph Loader @ 2010-12-28 23:01 UTC (permalink / raw)
To: Jiri Kosina, Greg Kroah-Hartman; +Cc: linux-kernel, Ralph Loader
Jiri Kosina <jkosina@suse.cz> wrote:
> Could you please split the staging bits and submit them separately to
> Greg?
As Jiri requested, the staging fixes as a separate commit. I'll repost the non-staging changes as a separate commit also.
Cheers,
Ralph.
staging: Fix some incorrect use of positive error codes.
Use -E... instead of just E... in a few places where negative error
codes are expected by a functions callers. These were found by grepping
with coccinelle & then inspecting by hand to determine which were bugs.
The staging/cxt1e1 driver appears to intentionally use positive E...
error codes in some places, and negative -E... error codes in others,
making it hard to know which is intended where - very likely I missed
some problems in that driver.
Signed-off-by: Ralph Loader <suckfish@ihug.co.nz>
---
drivers/staging/ath6kl/os/linux/ioctl.c | 10 +++++-----
drivers/staging/bcm/InterfaceMisc.c | 2 +-
drivers/staging/brcm80211/brcmfmac/wl_iw.c | 2 +-
drivers/staging/cxt1e1/linux.c | 2 +-
.../staging/westbridge/astoria/device/cyasdevice.c | 2 +-
5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/ath6kl/os/linux/ioctl.c b/drivers/staging/ath6kl/os/linux/ioctl.c
index d5f7ac0..0b10376 100644
--- a/drivers/staging/ath6kl/os/linux/ioctl.c
+++ b/drivers/staging/ath6kl/os/linux/ioctl.c
@@ -440,7 +440,7 @@ ar6000_ioctl_set_rssi_threshold(struct net_device *dev, struct ifreq *rq)
if (ar->rssi_map[j+1].rssi < ar->rssi_map[j].rssi) {
SWAP_THOLD(ar->rssi_map[j+1], ar->rssi_map[j]);
} else if (ar->rssi_map[j+1].rssi == ar->rssi_map[j].rssi) {
- return EFAULT;
+ return -EFAULT;
}
}
}
@@ -449,7 +449,7 @@ ar6000_ioctl_set_rssi_threshold(struct net_device *dev, struct ifreq *rq)
if (ar->rssi_map[j+1].rssi < ar->rssi_map[j].rssi) {
SWAP_THOLD(ar->rssi_map[j+1], ar->rssi_map[j]);
} else if (ar->rssi_map[j+1].rssi == ar->rssi_map[j].rssi) {
- return EFAULT;
+ return -EFAULT;
}
}
}
@@ -2870,7 +2870,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
gpio_output_set_cmd.enable_mask,
gpio_output_set_cmd.disable_mask);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
}
up(&ar->arSem);
@@ -2950,7 +2950,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
gpio_register_cmd.gpioreg_id,
gpio_register_cmd.value);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
/* Wait for acknowledgement from Target */
@@ -3041,7 +3041,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
} else {
ret = ar6000_gpio_intr_ack(dev, gpio_intr_ack_cmd.ack_mask);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
}
up(&ar->arSem);
diff --git a/drivers/staging/bcm/InterfaceMisc.c b/drivers/staging/bcm/InterfaceMisc.c
index 8fc893b..72be09f 100644
--- a/drivers/staging/bcm/InterfaceMisc.c
+++ b/drivers/staging/bcm/InterfaceMisc.c
@@ -102,7 +102,7 @@ InterfaceWRM(PS_INTERFACE_ADAPTER psIntfAdapter,
if((psIntfAdapter->psAdapter->StopAllXaction == TRUE) && (psIntfAdapter->psAdapter->chip_id >= T3LPB))
{
BCM_DEBUG_PRINT(psIntfAdapter->psAdapter,DBG_TYPE_OTHERS, WRM, DBG_LVL_ALL,"Currently Xaction is not allowed on the bus...");
- return EACCES;
+ return -EACCES;
}
if(psIntfAdapter->bSuspended ==TRUE || psIntfAdapter->bPreparingForBusSuspend == TRUE)
diff --git a/drivers/staging/brcm80211/brcmfmac/wl_iw.c b/drivers/staging/brcm80211/brcmfmac/wl_iw.c
index 979a494..b68bc54 100644
--- a/drivers/staging/brcm80211/brcmfmac/wl_iw.c
+++ b/drivers/staging/brcm80211/brcmfmac/wl_iw.c
@@ -1369,7 +1369,7 @@ wl_iw_iscan_set_scan(struct net_device *dev,
if (g_scan_specified_ssid) {
WL_TRACE(("%s Specific SCAN already running ignoring BC scan\n",
__func__));
- return EBUSY;
+ return -EBUSY;
}
memset(&ssid, 0, sizeof(ssid));
diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
index c793028..7af4887 100644
--- a/drivers/staging/cxt1e1/linux.c
+++ b/drivers/staging/cxt1e1/linux.c
@@ -548,7 +548,7 @@ do_set_port (struct net_device * ndev, void *data)
return -EINVAL; /* get card info */
if (pp.portnum >= ci->max_port) /* sanity check */
- return ENXIO;
+ return -ENXIO;
memcpy (&ci->port[pp.portnum].p, &pp, sizeof (struct sbecom_port_param));
return mkret (c4_set_port (ci, pp.portnum));
diff --git a/drivers/staging/westbridge/astoria/device/cyasdevice.c b/drivers/staging/westbridge/astoria/device/cyasdevice.c
index c76e383..0889730 100644
--- a/drivers/staging/westbridge/astoria/device/cyasdevice.c
+++ b/drivers/staging/westbridge/astoria/device/cyasdevice.c
@@ -389,7 +389,7 @@ EXPORT_SYMBOL(cyasdevice_gethaltag);
static int __init cyasdevice_init(void)
{
if (cyasdevice_initialize() != 0)
- return ENODEV;
+ return -ENODEV;
return 0;
}
--
1.7.3.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix some incorrect use of positive error codes.
2010-12-28 23:01 ` Ralph Loader
@ 2010-12-28 23:04 ` Ralph Loader
2011-01-21 0:05 ` Greg KH
1 sibling, 0 replies; 6+ messages in thread
From: Ralph Loader @ 2010-12-28 23:04 UTC (permalink / raw)
To: linux-kernel, trivial; +Cc: Jiri Kosina
[Reposting after staging fixes separated out, as requested.]
A few functions were incorrectly returning positive error code values
where negative error code values were expected.
fs/cifs/cifsencrypt.c: Trivial.
net/caif/cfrfml.c: err is correctly initialised to -EPROTO and then
later incorrectly set to EPROTO. So just delete the second
assignment.
arch/ppc/pseries/cmm.c: cmm_mem_going_offline() was returning a
positive error code to cmm_memory_cb() which then passed the value to
notifier_from_errno() which expects negative values. Resolve the
inconsistency by returning the 'notifier' value from
cmm_mem_going_offline() in the first place.
These were found using coccinelle to grep for returning positive EFOO,
and then going through the 300 or so hits to find a dozen or so that
were obviously wrong. XFS appears to intentionally use both positive
and negative error codes, I made no attempt to check that code for
consistency.
Signed-off-by: Ralph Loader <suckfish@ihug.co.nz>
---
arch/powerpc/platforms/pseries/cmm.c | 18 +++++-------------
fs/cifs/cifsencrypt.c | 2 +-
net/caif/cfrfml.c | 1 -
3 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
index f480386..3542fdc 100644
--- a/arch/powerpc/platforms/pseries/cmm.c
+++ b/arch/powerpc/platforms/pseries/cmm.c
@@ -526,7 +526,7 @@ static struct notifier_block cmm_mem_isolate_nb = {
* @arg: memory_notify structure with page range to be offlined
*
* Return value:
- * 0 on success
+ * NOTIFY_OK on success
**/
static int cmm_mem_going_offline(void *arg)
{
@@ -581,7 +581,7 @@ static int cmm_mem_going_offline(void *arg)
cmm_dbg("Failed to allocate memory for list "
"management. Memory hotplug "
"failed.\n");
- return ENOMEM;
+ return notifier_from_errno(-ENOMEM);
}
memcpy(npa, pa_curr, PAGE_SIZE);
if (pa_curr == cmm_page_list)
@@ -600,7 +600,7 @@ static int cmm_mem_going_offline(void *arg)
spin_unlock(&cmm_lock);
cmm_dbg("Released %ld pages in the search range.\n", freed);
- return 0;
+ return NOTIFY_OK;
}
/**
@@ -616,14 +616,11 @@ static int cmm_mem_going_offline(void *arg)
static int cmm_memory_cb(struct notifier_block *self,
unsigned long action, void *arg)
{
- int ret = 0;
-
switch (action) {
case MEM_GOING_OFFLINE:
mutex_lock(&hotplug_mutex);
hotplug_occurred = 1;
- ret = cmm_mem_going_offline(arg);
- break;
+ return cmm_mem_going_offline(arg);
case MEM_OFFLINE:
case MEM_CANCEL_OFFLINE:
mutex_unlock(&hotplug_mutex);
@@ -635,12 +632,7 @@ static int cmm_memory_cb(struct notifier_block *self,
break;
}
- if (ret)
- ret = notifier_from_errno(ret);
- else
- ret = NOTIFY_OK;
-
- return ret;
+ return NOTIFY_OK;
}
static struct notifier_block cmm_mem_nb = {
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index f856732..76df22b 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -585,7 +585,7 @@ setup_ntlmv2_rsp(struct cifsSesInfo *ses, const struct nls_table *nls_cp)
ses->auth_key.response = kmalloc(baselen + tilen, GFP_KERNEL);
if (!ses->auth_key.response) {
- rc = ENOMEM;
+ rc = -ENOMEM;
ses->auth_key.len = 0;
cERROR(1, "%s: Can't allocate auth blob", __func__);
goto setup_ntlmv2_rsp_ret;
diff --git a/net/caif/cfrfml.c b/net/caif/cfrfml.c
index e2fb5fa..f5531c6 100644
--- a/net/caif/cfrfml.c
+++ b/net/caif/cfrfml.c
@@ -162,7 +162,6 @@ static int cfrfml_receive(struct cflayer *layr, struct cfpkt *pkt)
tmppkt = NULL;
/* Verify that length is correct */
- err = EPROTO;
if (rfml->pdu_size != cfpkt_getlen(pkt) - RFM_HEAD_SIZE + 1)
goto out;
}
--
1.7.3.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix some incorrect use of positive error codes.
2010-12-28 23:01 ` Ralph Loader
2010-12-28 23:04 ` Ralph Loader
@ 2011-01-21 0:05 ` Greg KH
2011-01-21 6:27 ` Ralph Loader
1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2011-01-21 0:05 UTC (permalink / raw)
To: Ralph Loader; +Cc: Jiri Kosina, Greg Kroah-Hartman, linux-kernel
On Wed, Dec 29, 2010 at 12:01:16PM +1300, Ralph Loader wrote:
> Jiri Kosina <jkosina@suse.cz> wrote:
> > Could you please split the staging bits and submit them separately to
> > Greg?
>
> As Jiri requested, the staging fixes as a separate commit. I'll
> repost the non-staging changes as a separate commit also.
This doesn't seem to apply at the moment, care to redo it against the
latest linux-next tree?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix some incorrect use of positive error codes.
2011-01-21 0:05 ` Greg KH
@ 2011-01-21 6:27 ` Ralph Loader
0 siblings, 0 replies; 6+ messages in thread
From: Ralph Loader @ 2011-01-21 6:27 UTC (permalink / raw)
To: Greg KH; +Cc: Jiri Kosina, Greg Kroah-Hartman, linux-kernel
> This doesn't seem to apply at the moment, care to redo it against the
> latest linux-next tree?
>
> thanks,
>
> greg k-h
Patch against latest linux-next below.
Cheers,
Ralph.
>From e7e2289874f776dea2a684d218ba9554e7459599 Mon Sep 17 00:00:00 2001
From: Ralph Loader <suckfish@ihug.co.nz>
Date: Wed, 29 Dec 2010 11:12:15 +1300
Subject: [PATCH] staging: Fix some incorrect use of positive error codes.
Use -E... instead of just E... in a few places where negative error
codes are expected by a functions callers. These were found by grepping
with coccinelle & then inspecting by hand to determine which were bugs.
The staging/cxt1e1 driver appears to intentionally use positive E...
error codes in some places, and negative -E... error codes in others,
making it hard to know which is intended where - very likely I missed
some problems in that driver.
Signed-off-by: Ralph Loader <suckfish@ihug.co.nz>
---
drivers/staging/ath6kl/os/linux/ioctl.c | 10 +++++-----
drivers/staging/brcm80211/brcmfmac/wl_iw.c | 2 +-
drivers/staging/cxt1e1/linux.c | 2 +-
.../staging/westbridge/astoria/device/cyasdevice.c | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/ath6kl/os/linux/ioctl.c b/drivers/staging/ath6kl/os/linux/ioctl.c
index d5f7ac0..0b10376 100644
--- a/drivers/staging/ath6kl/os/linux/ioctl.c
+++ b/drivers/staging/ath6kl/os/linux/ioctl.c
@@ -440,7 +440,7 @@ ar6000_ioctl_set_rssi_threshold(struct net_device *dev, struct ifreq *rq)
if (ar->rssi_map[j+1].rssi < ar->rssi_map[j].rssi) {
SWAP_THOLD(ar->rssi_map[j+1], ar->rssi_map[j]);
} else if (ar->rssi_map[j+1].rssi == ar->rssi_map[j].rssi) {
- return EFAULT;
+ return -EFAULT;
}
}
}
@@ -449,7 +449,7 @@ ar6000_ioctl_set_rssi_threshold(struct net_device *dev, struct ifreq *rq)
if (ar->rssi_map[j+1].rssi < ar->rssi_map[j].rssi) {
SWAP_THOLD(ar->rssi_map[j+1], ar->rssi_map[j]);
} else if (ar->rssi_map[j+1].rssi == ar->rssi_map[j].rssi) {
- return EFAULT;
+ return -EFAULT;
}
}
}
@@ -2870,7 +2870,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
gpio_output_set_cmd.enable_mask,
gpio_output_set_cmd.disable_mask);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
}
up(&ar->arSem);
@@ -2950,7 +2950,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
gpio_register_cmd.gpioreg_id,
gpio_register_cmd.value);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
/* Wait for acknowledgement from Target */
@@ -3041,7 +3041,7 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
} else {
ret = ar6000_gpio_intr_ack(dev, gpio_intr_ack_cmd.ack_mask);
if (ret != A_OK) {
- ret = EIO;
+ ret = -EIO;
}
}
up(&ar->arSem);
diff --git a/drivers/staging/brcm80211/brcmfmac/wl_iw.c b/drivers/staging/brcm80211/brcmfmac/wl_iw.c
index c039ef1..b50e3c4 100644
--- a/drivers/staging/brcm80211/brcmfmac/wl_iw.c
+++ b/drivers/staging/brcm80211/brcmfmac/wl_iw.c
@@ -1368,7 +1368,7 @@ wl_iw_iscan_set_scan(struct net_device *dev,
if (g_scan_specified_ssid) {
WL_TRACE("%s Specific SCAN already running ignoring BC scan\n",
__func__);
- return EBUSY;
+ return -EBUSY;
}
memset(&ssid, 0, sizeof(ssid));
diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c
index 0f78f89..9ced08f 100644
--- a/drivers/staging/cxt1e1/linux.c
+++ b/drivers/staging/cxt1e1/linux.c
@@ -548,7 +548,7 @@ do_set_port (struct net_device * ndev, void *data)
return -EINVAL; /* get card info */
if (pp.portnum >= ci->max_port) /* sanity check */
- return ENXIO;
+ return -ENXIO;
memcpy (&ci->port[pp.portnum].p, &pp, sizeof (struct sbecom_port_param));
return mkret (c4_set_port (ci, pp.portnum));
diff --git a/drivers/staging/westbridge/astoria/device/cyasdevice.c b/drivers/staging/westbridge/astoria/device/cyasdevice.c
index c76e383..0889730 100644
--- a/drivers/staging/westbridge/astoria/device/cyasdevice.c
+++ b/drivers/staging/westbridge/astoria/device/cyasdevice.c
@@ -389,7 +389,7 @@ EXPORT_SYMBOL(cyasdevice_gethaltag);
static int __init cyasdevice_init(void)
{
if (cyasdevice_initialize() != 0)
- return ENODEV;
+ return -ENODEV;
return 0;
}
--
1.7.3.4
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-21 6:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-26 5:57 [PATCH] Fix some incorrect use of positive error codes Ralph Loader
2010-12-27 11:27 ` Jiri Kosina
2010-12-28 23:01 ` Ralph Loader
2010-12-28 23:04 ` Ralph Loader
2011-01-21 0:05 ` Greg KH
2011-01-21 6:27 ` Ralph Loader
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).