LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jiri Kosina <jkosina@suse.cz>
To: Ralph Loader <suckfish@ihug.co.nz>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Fix some incorrect use of positive error codes.
Date: Mon, 27 Dec 2010 12:27:56 +0100 (CET)	[thread overview]
Message-ID: <alpine.LNX.2.00.1012271227260.16569@pobox.suse.cz> (raw)
In-Reply-To: <20101226185718.aa60e0cd.suckfish@ihug.co.nz>

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.

  reply	other threads:[~2010-12-27 11:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-26  5:57 Ralph Loader
2010-12-27 11:27 ` Jiri Kosina [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LNX.2.00.1012271227260.16569@pobox.suse.cz \
    --to=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=suckfish@ihug.co.nz \
    --subject='Re: [PATCH] Fix some incorrect use of positive error codes.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).