[PATCH 00/14] Modify signed comparisons of unsigned variables

[PATCH 00/14] Modify signed comparisons of unsigned variables

[Tushar Behera]

The occurrences were identified through the coccinelle script at
following location.

http://www.emn.fr/z-info/coccinelle/rules/find_unsigned.cocci

Signed checks for unsigned variables are removed if it is also checked
for upper error limit. For error checks, IS_ERR_VALUE() macros is used.

Tushar Behera (14):
  [media] ivtv: Remove redundant check on unsigned variable
  [media] meye: Remove redundant check on unsigned variable
  [media] saa7134: Remove redundant check on unsigned variable
  [media] tlg2300: Remove redundant check on unsigned variable
  [media] atmel-isi: Update error check for unsigned variables
  pinctrl: samsung: Update error check for unsigned variables
  pinctrl: SPEAr: Update error check for unsigned variables
  xen: netback: Remove redundant check on unsigned variable
  xen: events: Remove redundant check on unsigned variable
  atm: Removed redundant check on unsigned variable
  HID: hiddev: Remove redundant check on unsigned variable
  gru: Remove redundant check on unsigned variable
  misc: tsl2550: Remove redundant check on unsigned variable
  wlcore: Remove redundant check on unsigned variable

 drivers/atm/fore200e.c                        |    2 +-
 drivers/hid/usbhid/hiddev.c                   |    2 +-
 drivers/media/pci/ivtv/ivtv-ioctl.c           |    2 +-
 drivers/media/pci/meye/meye.c                 |    2 +-
 drivers/media/pci/saa7134/saa7134-video.c     |    2 +-
 drivers/media/platform/soc_camera/atmel-isi.c |    2 +-
 drivers/media/usb/tlg2300/pd-video.c          |    2 +-
 drivers/misc/sgi-gru/grukdump.c               |    2 +-
 drivers/misc/tsl2550.c                        |    4 ++--
 drivers/net/wireless/ti/wlcore/debugfs.c      |    2 +-
 drivers/net/xen-netback/netback.c             |    4 ++--
 drivers/pinctrl/pinctrl-samsung.c             |    2 +-
 drivers/pinctrl/spear/pinctrl-plgpio.c        |    2 +-
 drivers/xen/events.c                          |    2 +-
 14 files changed, 16 insertions(+), 16 deletions(-)

-- 
1.7.4.1

CC: Mauro Carvalho Chehab <mchehab@infradead.org>
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Jeremy Fitzhardinge <jeremy@goop.org>
CC: Chas Williams <chas@cmf.nrl.navy.mil>
CC: Jack Steiner <steiner@sgi.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Luciano Coelho <coelho@ti.com>
CC: Jiri Kosina <jkosina@suse.cz>
CC: ivtv-devel@ivtvdriver.org
CC: linux-media@vger.kernel.org
CC: xen-devel@lists.xensource.com
CC: netdev@vger.kernel.org
CC: virtualization@lists.linux-foundation.org
CC: linux-atm-general@lists.sourceforge.net
CC: linux-usb@vger.kernel.org
CC: linux-input@vger.kernel.org
CC: linux-wireless@vger.kernel.org

[PATCH 01/14] [media] ivtv: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Mauro Carvalho Chehab <mchehab@infradead.org>
CC: ivtv-devel@ivtvdriver.org
CC: linux-media@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/media/pci/ivtv/ivtv-ioctl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c b/drivers/media/pci/ivtv/ivtv-ioctl.c
index 949ae23..4b47b5c 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.c
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
@@ -993,7 +993,7 @@ int ivtv_s_input(struct file *file, void *fh, unsigned int inp)
 	v4l2_std_id std;
 	int i;
 
-	if (inp < 0 || inp >= itv->nof_inputs)
+	if (inp >= itv->nof_inputs)
 		return -EINVAL;
 
 	if (inp == itv->active_input) {
-- 
1.7.4.1

[PATCH 02/14] [media] meye: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Mauro Carvalho Chehab <mchehab@infradead.org>
CC: linux-media@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/media/pci/meye/meye.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/pci/meye/meye.c b/drivers/media/pci/meye/meye.c
index e5a76da..ae7d320 100644
--- a/drivers/media/pci/meye/meye.c
+++ b/drivers/media/pci/meye/meye.c
@@ -1945,7 +1945,7 @@ static struct pci_driver meye_driver = {
 static int __init meye_init(void)
 {
 	gbuffers = max(2, min((int)gbuffers, MEYE_MAX_BUFNBRS));
-	if (gbufsize < 0 || gbufsize > MEYE_MAX_BUFSIZE)
+	if (gbufsize > MEYE_MAX_BUFSIZE)
 		gbufsize = MEYE_MAX_BUFSIZE;
 	gbufsize = PAGE_ALIGN(gbufsize);
 	printk(KERN_INFO "meye: using %d buffers with %dk (%dk total) "
-- 
1.7.4.1

[PATCH 03/14] [media] saa7134: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether the unsigned variable is less than 0.

CC: Mauro Carvalho Chehab <mchehab@infradead.org>
CC: linux-media@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/media/pci/saa7134/saa7134-video.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 4a77124..3abf527 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2511,7 +2511,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
 	/* sanitycheck insmod options */
 	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
 		gbuffers = 2;
-	if (gbufsize < 0 || gbufsize > gbufsize_max)
+	if (gbufsize > gbufsize_max)
 		gbufsize = gbufsize_max;
 	gbufsize = (gbufsize + PAGE_SIZE - 1) & PAGE_MASK;
 
-- 
1.7.4.1

[PATCH 04/14] [media] tlg2300: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Mauro Carvalho Chehab <mchehab@infradead.org>
CC: linux-media@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/media/usb/tlg2300/pd-video.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/usb/tlg2300/pd-video.c b/drivers/media/usb/tlg2300/pd-video.c
index 1f448ac..dd157e7 100644
--- a/drivers/media/usb/tlg2300/pd-video.c
+++ b/drivers/media/usb/tlg2300/pd-video.c
@@ -923,7 +923,7 @@ static int vidioc_s_input(struct file *file, void *fh, unsigned int i)
 	struct poseidon *pd = front->pd;
 	s32 ret, cmd_status;
 
-	if (i < 0 || i >= POSEIDON_INPUTS)
+	if (i >= POSEIDON_INPUTS)
 		return -EINVAL;
 	ret = send_set_req(pd, SGNL_SRC_SEL,
 			pd_inputs[i].tlg_src, &cmd_status);
-- 
1.7.4.1

[PATCH 05/14] [media] atmel-isi: Update error check for unsigned variables

[Tushar Behera]

Checking '< 0' for unsigned variables always returns false. For error
codes, use IS_ERR_VALUE() instead.

CC: Mauro Carvalho Chehab <mchehab@infradead.org>
CC: linux-media@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/media/platform/soc_camera/atmel-isi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 6274a91..5bd65df 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -1020,7 +1020,7 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
+	if (IS_ERR_VALUE(irq)) {
 		ret = irq;
 		goto err_req_irq;
 	}
-- 
1.7.4.1

[PATCH 06/14] pinctrl: samsung: Update error check for unsigned variables

[Tushar Behera]

Checking '< 0' for unsigned variables always returns false. For error
codes, use IS_ERR_VALUE() instead.

CC: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/pinctrl/pinctrl-samsung.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
index 81c9896..3b52c17 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -560,7 +560,7 @@ static int __devinit samsung_pinctrl_parse_dt_pins(struct platform_device *pdev,
 	const char *pin_name;
 
 	*npins = of_property_count_strings(cfg_np, "samsung,pins");
-	if (*npins < 0) {
+	if (IS_ERR_VALUE(*npins)) {
 		dev_err(dev, "invalid pin list in %s node", cfg_np->name);
 		return -EINVAL;
 	}
-- 
1.7.4.1

[PATCH 07/14] pinctrl: SPEAr: Update error check for unsigned variables

[Tushar Behera]

Checking '< 0' for unsigned variables always returns false. For error
codes, use IS_ERR_VALUE() instead.

CC: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/pinctrl/spear/pinctrl-plgpio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/pinctrl/spear/pinctrl-plgpio.c b/drivers/pinctrl/spear/pinctrl-plgpio.c
index 1044ad3..9e0c731 100644
--- a/drivers/pinctrl/spear/pinctrl-plgpio.c
+++ b/drivers/pinctrl/spear/pinctrl-plgpio.c
@@ -283,7 +283,7 @@ static int plgpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	struct plgpio *plgpio = container_of(chip, struct plgpio, chip);
 
-	if (plgpio->irq_base < 0)
+	if (IS_ERR_VALUE(plgpio->irq_base))
 		return -EINVAL;
 
 	return irq_find_mapping(plgpio->irq_domain, offset);
-- 
1.7.4.1

[PATCH 08/14] xen: netback: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Ian Campbell <ian.campbell@citrix.com>
CC: xen-devel@lists.xensource.com
CC: netdev@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/net/xen-netback/netback.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index aab8677..515e10c 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -190,14 +190,14 @@ static int get_page_ext(struct page *pg,
 
 	group = ext.e.group - 1;
 
-	if (group < 0 || group >= xen_netbk_group_nr)
+	if (group >= xen_netbk_group_nr)
 		return 0;
 
 	netbk = &xen_netbk[group];
 
 	idx = ext.e.idx;
 
-	if ((idx < 0) || (idx >= MAX_PENDING_REQS))
+	if (idx >= MAX_PENDING_REQS)
 		return 0;
 
 	if (netbk->mmap_pages[idx] != pg)
-- 
1.7.4.1

[PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Jeremy Fitzhardinge <jeremy@goop.org>
CC: xen-devel@lists.xensource.com
CC: virtualization@lists.linux-foundation.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/xen/events.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 4293c57..cadd7d1 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -216,7 +216,7 @@ static void xen_irq_info_pirq_init(unsigned irq,
  */
 static unsigned int evtchn_from_irq(unsigned irq)
 {
-	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq)))
+	if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
 		return 0;
 
 	return info_for_irq(irq)->evtchn;
-- 
1.7.4.1

[PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Jeremy Fitzhardinge <jeremy@goop.org>
CC: xen-devel@lists.xensource.com
CC: virtualization@lists.linux-foundation.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/xen/events.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 4293c57..cadd7d1 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -216,7 +216,7 @@ static void xen_irq_info_pirq_init(unsigned irq,
  */
 static unsigned int evtchn_from_irq(unsigned irq)
 {
-	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq)))
+	if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
 		return 0;
 
 	return info_for_irq(irq)->evtchn;
-- 
1.7.4.1

[PATCH 10/14] atm: Removed redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Chas Williams <chas@cmf.nrl.navy.mil>
CC: linux-atm-general@lists.sourceforge.net
CC: netdev@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/atm/fore200e.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
index 361f5ae..fdd3fe7 100644
--- a/drivers/atm/fore200e.c
+++ b/drivers/atm/fore200e.c
@@ -972,7 +972,7 @@ int bsq_audit(int where, struct host_bsq* bsq, int scheme, int magn)
 		   where, scheme, magn, buffer->index, buffer->scheme);
 	}
 
-	if ((buffer->index < 0) || (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ])) {
+	if (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ]) {
 	    printk(FORE200E "bsq_audit(%d): queue %d.%d, out of range buffer index = %ld !\n",
 		   where, scheme, magn, buffer->index);
 	}
-- 
1.7.4.1

[PATCH 11/14] HID: hiddev: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Jiri Kosina <jkosina@suse.cz>
CC: linux-usb@vger.kernel.org
CC: linux-input@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/hid/usbhid/hiddev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 14599e2..711c965 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -625,7 +625,7 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 
 	case HIDIOCAPPLICATION:
-		if (arg < 0 || arg >= hid->maxapplication)
+		if (arg >= hid->maxapplication)
 			break;
 
 		for (i = 0; i < hid->maxcollection; i++)
-- 
1.7.4.1

[PATCH 12/14] gru: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Jack Steiner <steiner@sgi.com>
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/misc/sgi-gru/grukdump.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/sgi-gru/grukdump.c b/drivers/misc/sgi-gru/grukdump.c
index 9b2062d..860733b 100644
--- a/drivers/misc/sgi-gru/grukdump.c
+++ b/drivers/misc/sgi-gru/grukdump.c
@@ -197,7 +197,7 @@ int gru_dump_chiplet_request(unsigned long arg)
 		return -EFAULT;
 
 	/* Currently, only dump by gid is implemented */
-	if (req.gid >= gru_max_gids || req.gid < 0)
+	if (req.gid >= gru_max_gids)
 		return -EINVAL;
 
 	gru = GID_TO_GRU(req.gid);
-- 
1.7.4.1

[PATCH 13/14] misc: tsl2550: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/misc/tsl2550.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/tsl2550.c b/drivers/misc/tsl2550.c
index 0beb298..0438569 100644
--- a/drivers/misc/tsl2550.c
+++ b/drivers/misc/tsl2550.c
@@ -204,7 +204,7 @@ static ssize_t tsl2550_store_power_state(struct device *dev,
 	unsigned long val = simple_strtoul(buf, NULL, 10);
 	int ret;
 
-	if (val < 0 || val > 1)
+	if (val > 1)
 		return -EINVAL;
 
 	mutex_lock(&data->update_lock);
@@ -236,7 +236,7 @@ static ssize_t tsl2550_store_operating_mode(struct device *dev,
 	unsigned long val = simple_strtoul(buf, NULL, 10);
 	int ret;
 
-	if (val < 0 || val > 1)
+	if (val > 1)
 		return -EINVAL;
 
 	if (data->power_state == 0)
-- 
1.7.4.1

[PATCH 14/14] wlcore: Remove redundant check on unsigned variable

[Tushar Behera]

No need to check whether unsigned variable is less than 0.

CC: Luciano Coelho <coelho@ti.com>
CC: linux-wireless@vger.kernel.org
CC: netdev@vger.kernel.org
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/net/wireless/ti/wlcore/debugfs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/debugfs.c b/drivers/net/wireless/ti/wlcore/debugfs.c
index c86bb00..93f801d 100644
--- a/drivers/net/wireless/ti/wlcore/debugfs.c
+++ b/drivers/net/wireless/ti/wlcore/debugfs.c
@@ -993,7 +993,7 @@ static ssize_t sleep_auth_write(struct file *file,
 		return -EINVAL;
 	}
 
-	if (value < 0 || value > WL1271_PSM_MAX) {
+	if (value > WL1271_PSM_MAX) {
 		wl1271_warning("sleep_auth must be between 0 and %d",
 			       WL1271_PSM_MAX);
 		return -ERANGE;
-- 
1.7.4.1

Re: [PATCH 07/14] pinctrl: SPEAr: Update error check for unsigned variables

[viresh kumar]

On Fri, Nov 16, 2012 at 12:20 PM, Tushar Behera
<tushar.behera@linaro.org> wrote:
> Checking '< 0' for unsigned variables always returns false. For error
> codes, use IS_ERR_VALUE() instead.
>
> CC: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/pinctrl/spear/pinctrl-plgpio.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pinctrl/spear/pinctrl-plgpio.c b/drivers/pinctrl/spear/pinctrl-plgpio.c
> index 1044ad3..9e0c731 100644
> --- a/drivers/pinctrl/spear/pinctrl-plgpio.c
> +++ b/drivers/pinctrl/spear/pinctrl-plgpio.c
> @@ -283,7 +283,7 @@ static int plgpio_to_irq(struct gpio_chip *chip, unsigned offset)
>  {
>         struct plgpio *plgpio = container_of(chip, struct plgpio, chip);
>
> -       if (plgpio->irq_base < 0)
> +       if (IS_ERR_VALUE(plgpio->irq_base))
>                 return -EINVAL;
>
>         return irq_find_mapping(plgpio->irq_domain, offset);

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH 13/14] misc: tsl2550: Remove redundant check on unsigned variable

[Arnd Bergmann]

On Friday 16 November 2012, Tushar Behera wrote:
> No need to check whether unsigned variable is less than 0.
> 
> CC: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 08/14] xen: netback: Remove redundant check on unsigned variable

[Ian Campbell]

On Fri, 2012-11-16 at 06:50 +0000, Tushar Behera wrote:
> No need to check whether unsigned variable is less than 0.
> 
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: xen-devel@lists.xensource.com
> CC: netdev@vger.kernel.org
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks.

> ---
>  drivers/net/xen-netback/netback.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index aab8677..515e10c 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -190,14 +190,14 @@ static int get_page_ext(struct page *pg,
>  
>  	group = ext.e.group - 1;
>  
> -	if (group < 0 || group >= xen_netbk_group_nr)
> +	if (group >= xen_netbk_group_nr)
>  		return 0;
>  
>  	netbk = &xen_netbk[group];
>  
>  	idx = ext.e.idx;
>  
> -	if ((idx < 0) || (idx >= MAX_PENDING_REQS))
> +	if (idx >= MAX_PENDING_REQS)
>  		return 0;
>  
>  	if (netbk->mmap_pages[idx] != pg)

Re: [PATCH 11/14] HID: hiddev: Remove redundant check on unsigned variable

[Jiri Kosina]

On Fri, 16 Nov 2012, Tushar Behera wrote:

> No need to check whether unsigned variable is less than 0.
> 
> CC: Jiri Kosina <jkosina@suse.cz>
> CC: linux-usb@vger.kernel.org
> CC: linux-input@vger.kernel.org
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/hid/usbhid/hiddev.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 14599e2..711c965 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -625,7 +625,7 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		break;
>  
>  	case HIDIOCAPPLICATION:
> -		if (arg < 0 || arg >= hid->maxapplication)
> +		if (arg >= hid->maxapplication)
>  			break;
>  
>  		for (i = 0; i < hid->maxcollection; i++)

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Konrad Rzeszutek Wilk]

On Fri, Nov 16, 2012 at 12:20:41PM +0530, Tushar Behera wrote:
> No need to check whether unsigned variable is less than 0.
> 
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> CC: Jeremy Fitzhardinge <jeremy@goop.org>
> CC: xen-devel@lists.xensource.com
> CC: virtualization@lists.linux-foundation.org
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/xen/events.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 4293c57..cadd7d1 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -216,7 +216,7 @@ static void xen_irq_info_pirq_init(unsigned irq,
>   */
>  static unsigned int evtchn_from_irq(unsigned irq)
>  {
> -	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq)))
> +	if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
>  		return 0;
>  
>  	return info_for_irq(irq)->evtchn;
> -- 
> 1.7.4.1

Re: [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Konrad Rzeszutek Wilk]

On Fri, Nov 16, 2012 at 12:20:41PM +0530, Tushar Behera wrote:
> No need to check whether unsigned variable is less than 0.
> 
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> CC: Jeremy Fitzhardinge <jeremy@goop.org>
> CC: xen-devel@lists.xensource.com
> CC: virtualization@lists.linux-foundation.org
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/xen/events.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 4293c57..cadd7d1 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -216,7 +216,7 @@ static void xen_irq_info_pirq_init(unsigned irq,
>   */
>  static unsigned int evtchn_from_irq(unsigned irq)
>  {
> -	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n", irq)))
> +	if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
>  		return 0;
>  
>  	return info_for_irq(irq)->evtchn;
> -- 
> 1.7.4.1

Re: [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Jeremy Fitzhardinge]

[-- Attachment #1.1: Type: text/plain, Size: 1501 bytes --]

To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).

J

Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

>On Fri, Nov 16, 2012 at 12:20:41PM +0530, Tushar Behera wrote:
>> No need to check whether unsigned variable is less than 0.
>> 
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
>Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
>> CC: Jeremy Fitzhardinge <jeremy@goop.org>
>> CC: xen-devel@lists.xensource.com
>> CC: virtualization@lists.linux-foundation.org
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> ---
>>  drivers/xen/events.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
>> index 4293c57..cadd7d1 100644
>> --- a/drivers/xen/events.c
>> +++ b/drivers/xen/events.c
>> @@ -216,7 +216,7 @@ static void xen_irq_info_pirq_init(unsigned irq,
>>   */
>>  static unsigned int evtchn_from_irq(unsigned irq)
>>  {
>> -	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n",
>irq)))
>> +	if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
>>  		return 0;
>>  
>>  	return info_for_irq(irq)->evtchn;
>> -- 
>> 1.7.4.1

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

[-- Attachment #1.2: Type: text/html, Size: 2231 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Jeremy Fitzhardinge]

[-- Attachment #1.1: Type: text/plain, Size: 1462 bytes --]

To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).

J

Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

>On Fri, Nov 16, 2012 at 12:20:41PM +0530, Tushar Behera wrote:
>> No need to check whether unsigned variable is less than 0.
>> 
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
>Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
>> CC: Jeremy Fitzhardinge <jeremy@goop.org>
>> CC: xen-devel@lists.xensource.com
>> CC: virtualization@lists.linux-foundation.org
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> ---
>>  drivers/xen/events.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
>> index 4293c57..cadd7d1 100644
>> --- a/drivers/xen/events.c
>> +++ b/drivers/xen/events.c
>> @@ -216,7 +216,7 @@ static void xen_irq_info_pirq_init(unsigned irq,
>>   */
>>  static unsigned int evtchn_from_irq(unsigned irq)
>>  {
>> -	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n",
>irq)))
>> +	if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
>>  		return 0;
>>  
>>  	return info_for_irq(irq)->evtchn;
>> -- 
>> 1.7.4.1

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

[-- Attachment #1.2: Type: text/html, Size: 2224 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 14/14] wlcore: Remove redundant check on unsigned variable

[Luciano Coelho]

On Fri, 2012-11-16 at 12:20 +0530, Tushar Behera wrote:
> No need to check whether unsigned variable is less than 0.
> 
> CC: Luciano Coelho <coelho@ti.com>
> CC: linux-wireless@vger.kernel.org
> CC: netdev@vger.kernel.org
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---

Applied in the wl12xx.git tree.  Thanks!

--
Luca.

Re: [PATCH 06/14] pinctrl: samsung: Update error check for unsigned variables

[Linus Walleij]

On Fri, Nov 16, 2012 at 7:50 AM, Tushar Behera <tushar.behera@linaro.org> wrote:

> Checking '< 0' for unsigned variables always returns false. For error
> codes, use IS_ERR_VALUE() instead.
>
> CC: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/pinctrl/pinctrl-samsung.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
> index 81c9896..3b52c17 100644
> --- a/drivers/pinctrl/pinctrl-samsung.c
> +++ b/drivers/pinctrl/pinctrl-samsung.c
> @@ -560,7 +560,7 @@ static int __devinit samsung_pinctrl_parse_dt_pins(struct platform_device *pdev,
>         const char *pin_name;
>
>         *npins = of_property_count_strings(cfg_np, "samsung,pins");
> -       if (*npins < 0) {
> +       if (IS_ERR_VALUE(*npins)) {
>                 dev_err(dev, "invalid pin list in %s node", cfg_np->name);
>                 return -EINVAL;
>         }

Acked-by: Linus Walleij <linus.walleij@linaro.org>

For this merge window Kukjin is handling the Samsung pinctrl patches,
Kukjin can you pick this to your tree to avoid any mess?

Yours,
Linus Walleij

Re: [PATCH 05/14] [media] atmel-isi: Update error check for unsigned variables

[Guennadi Liakhovetski]

On Fri, 16 Nov 2012, Tushar Behera wrote:

> Checking '< 0' for unsigned variables always returns false. For error
> codes, use IS_ERR_VALUE() instead.

Wouldn't just changing "irq" type to "int" also work? I think that would 
be a more straight-forward solution. If however there are strong arguments 
against that, I'm fine with this fix too.

Thanks
Guennadi

> 
> CC: Mauro Carvalho Chehab <mchehab@infradead.org>
> CC: linux-media@vger.kernel.org
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/media/platform/soc_camera/atmel-isi.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
> index 6274a91..5bd65df 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -1020,7 +1020,7 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
>  	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
>  
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> +	if (IS_ERR_VALUE(irq)) {
>  		ret = irq;
>  		goto err_req_irq;
>  	}
> -- 
> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

RE: [PATCH 06/14] pinctrl: samsung: Update error check for unsigned variables

[Kukjin Kim]

Linus Walleij wrote:
> 
> On Fri, Nov 16, 2012 at 7:50 AM, Tushar Behera <tushar.behera@linaro.org>
> wrote:
> 
> > Checking '< 0' for unsigned variables always returns false. For error
> > codes, use IS_ERR_VALUE() instead.
> >
> > CC: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> > ---
> >  drivers/pinctrl/pinctrl-samsung.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-samsung.c
> b/drivers/pinctrl/pinctrl-samsung.c
> > index 81c9896..3b52c17 100644
> > --- a/drivers/pinctrl/pinctrl-samsung.c
> > +++ b/drivers/pinctrl/pinctrl-samsung.c
> > @@ -560,7 +560,7 @@ static int __devinit
> samsung_pinctrl_parse_dt_pins(struct platform_device *pdev,
> >         const char *pin_name;
> >
> >         *npins = of_property_count_strings(cfg_np, "samsung,pins");
> > -       if (*npins < 0) {
> > +       if (IS_ERR_VALUE(*npins)) {
> >                 dev_err(dev, "invalid pin list in %s node",
cfg_np->name);
> >                 return -EINVAL;
> >         }
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> For this merge window Kukjin is handling the Samsung pinctrl patches,
> Kukjin can you pick this to your tree to avoid any mess?
> 
Sure, I will.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Tushar Behera]

On 11/16/2012 10:23 PM, Jeremy Fitzhardinge wrote:
> To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).
> 
> J
> 

The check is on the function argument which is unsigned, so checking '<
0' doesn't make sense. We should force signed check only if the argument
is of signed type. In any case, even if irq has been assigned some error
value, that would be caught by the check irq >= nr_irqs.

> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
>> On Fri, Nov 16, 2012 at 12:20:41PM +0530, Tushar Behera wrote:
>>> No need to check whether unsigned variable is less than 0.
>>>
>>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>
>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>
>>> CC: Jeremy Fitzhardinge <jeremy@goop.org>
>>> CC: xen-devel@lists.xensource.com
>>> CC: virtualization@lists.linux-foundation.org
>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>> ---
>>>  drivers/xen/events.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
>>> index 4293c57..cadd7d1 100644
>>> --- a/drivers/xen/events.c
>>> +++ b/drivers/xen/events.c
>>> @@ -216,7 +216,7 @@ static void xen_irq_info_pirq_init(unsigned irq,
>>>   */
>>>  static unsigned int evtchn_from_irq(unsigned irq)
>>>  {
>>> -	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n",
>> irq)))
>>> +	if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
>>>  		return 0;
>>>  
>>>  	return info_for_irq(irq)->evtchn;
>>> -- 
>>> 1.7.4.1
> 


-- 
Tushar Behera

Re: [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Tushar Behera]

On 11/16/2012 10:23 PM, Jeremy Fitzhardinge wrote:
> To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).
> 
> J
> 

The check is on the function argument which is unsigned, so checking '<
0' doesn't make sense. We should force signed check only if the argument
is of signed type. In any case, even if irq has been assigned some error
value, that would be caught by the check irq >= nr_irqs.

> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
>> On Fri, Nov 16, 2012 at 12:20:41PM +0530, Tushar Behera wrote:
>>> No need to check whether unsigned variable is less than 0.
>>>
>>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>
>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>
>>> CC: Jeremy Fitzhardinge <jeremy@goop.org>
>>> CC: xen-devel@lists.xensource.com
>>> CC: virtualization@lists.linux-foundation.org
>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>> ---
>>>  drivers/xen/events.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
>>> index 4293c57..cadd7d1 100644
>>> --- a/drivers/xen/events.c
>>> +++ b/drivers/xen/events.c
>>> @@ -216,7 +216,7 @@ static void xen_irq_info_pirq_init(unsigned irq,
>>>   */
>>>  static unsigned int evtchn_from_irq(unsigned irq)
>>>  {
>>> -	if (unlikely(WARN(irq < 0 || irq >= nr_irqs, "Invalid irq %d!\n",
>> irq)))
>>> +	if (unlikely(WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)))
>>>  		return 0;
>>>  
>>>  	return info_for_irq(irq)->evtchn;
>>> -- 
>>> 1.7.4.1
> 


-- 
Tushar Behera

Re: [PATCH 05/14] [media] atmel-isi: Update error check for unsigned variables

[Tushar Behera]

On 11/18/2012 04:46 AM, Guennadi Liakhovetski wrote:
> On Fri, 16 Nov 2012, Tushar Behera wrote:
> 
>> Checking '< 0' for unsigned variables always returns false. For error
>> codes, use IS_ERR_VALUE() instead.
> 
> Wouldn't just changing "irq" type to "int" also work? I think that would 
> be a more straight-forward solution. If however there are strong arguments 
> against that, I'm fine with this fix too.
> 

By changing irq to signed variable, we would get compilation warning in
subsequent line (request_irq).

> Thanks
> Guennadi
> 
>>
>> CC: Mauro Carvalho Chehab <mchehab@infradead.org>
>> CC: linux-media@vger.kernel.org
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> ---
>>  drivers/media/platform/soc_camera/atmel-isi.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>> index 6274a91..5bd65df 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -1020,7 +1020,7 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
>>  	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
>>  
>>  	irq = platform_get_irq(pdev, 0);
>> -	if (irq < 0) {
>> +	if (IS_ERR_VALUE(irq)) {
>>  		ret = irq;
>>  		goto err_req_irq;
>>  	}
>> -- 
>> 1.7.4.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 


-- 
Tushar Behera

Re: [Xen-devel] [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Ian Campbell]

On Mon, 2012-11-19 at 03:52 +0000, Tushar Behera wrote:
> On 11/16/2012 10:23 PM, Jeremy Fitzhardinge wrote:
> > To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).
> > 
> > J
> > 
> 
> The check is on the function argument which is unsigned, so checking '<
> 0' doesn't make sense. We should force signed check only if the argument
> is of signed type. In any case, even if irq has been assigned some error
> value, that would be caught by the check irq >= nr_irqs.

Jeremy is (I think) arguing that this check is not redundant because
someone might change the type of the argument to be signed and until
then the compiler can trivially optimise the check away, so what's the
harm in it?

I'm somewhat inclined to agree with him.

Ian.

Re: [Xen-devel] [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Ian Campbell]

On Mon, 2012-11-19 at 03:52 +0000, Tushar Behera wrote:
> On 11/16/2012 10:23 PM, Jeremy Fitzhardinge wrote:
> > To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).
> > 
> > J
> > 
> 
> The check is on the function argument which is unsigned, so checking '<
> 0' doesn't make sense. We should force signed check only if the argument
> is of signed type. In any case, even if irq has been assigned some error
> value, that would be caught by the check irq >= nr_irqs.

Jeremy is (I think) arguing that this check is not redundant because
someone might change the type of the argument to be signed and until
then the compiler can trivially optimise the check away, so what's the
harm in it?

I'm somewhat inclined to agree with him.

Ian.

Re: [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Ian Campbell]

On Mon, 2012-11-19 at 03:52 +0000, Tushar Behera wrote:
> On 11/16/2012 10:23 PM, Jeremy Fitzhardinge wrote:
> > To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).
> > 
> > J
> > 
> 
> The check is on the function argument which is unsigned, so checking '<
> 0' doesn't make sense. We should force signed check only if the argument
> is of signed type. In any case, even if irq has been assigned some error
> value, that would be caught by the check irq >= nr_irqs.

Jeremy is (I think) arguing that this check is not redundant because
someone might change the type of the argument to be signed and until
then the compiler can trivially optimise the check away, so what's the
harm in it?

I'm somewhat inclined to agree with him.

Ian.

Re: [Xen-devel] [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Tushar Behera]

On 11/19/2012 04:01 PM, Ian Campbell wrote:
> On Mon, 2012-11-19 at 03:52 +0000, Tushar Behera wrote:
>> On 11/16/2012 10:23 PM, Jeremy Fitzhardinge wrote:
>>> To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).
>>>
>>> J
>>>
>>
>> The check is on the function argument which is unsigned, so checking '<
>> 0' doesn't make sense. We should force signed check only if the argument
>> is of signed type. In any case, even if irq has been assigned some error
>> value, that would be caught by the check irq >= nr_irqs.
> 
> Jeremy is (I think) arguing that this check is not redundant because
> someone might change the type of the argument to be signed and until
> then the compiler can trivially optimise the check away, so what's the
> harm in it?
> 
> I'm somewhat inclined to agree with him.
> 
> Ian.
> 
Ok, I don't have much argument against this.

-- 
Tushar Behera

Re: [Xen-devel] [PATCH 09/14] xen: events: Remove redundant check on unsigned variable

[Tushar Behera]

On 11/19/2012 04:01 PM, Ian Campbell wrote:
> On Mon, 2012-11-19 at 03:52 +0000, Tushar Behera wrote:
>> On 11/16/2012 10:23 PM, Jeremy Fitzhardinge wrote:
>>> To be honest I'd nack this kind of patch. The test is only redundant in the most trivial sense that the compiler can easily optimise away. The point of the test is to make sure that the range is OK even if the type subsequently becomes signed (to hold a -ve error, for example).
>>>
>>> J
>>>
>>
>> The check is on the function argument which is unsigned, so checking '<
>> 0' doesn't make sense. We should force signed check only if the argument
>> is of signed type. In any case, even if irq has been assigned some error
>> value, that would be caught by the check irq >= nr_irqs.
> 
> Jeremy is (I think) arguing that this check is not redundant because
> someone might change the type of the argument to be signed and until
> then the compiler can trivially optimise the check away, so what's the
> harm in it?
> 
> I'm somewhat inclined to agree with him.
> 
> Ian.
> 
Ok, I don't have much argument against this.

-- 
Tushar Behera

Re: [PATCH 07/14] pinctrl: SPEAr: Update error check for unsigned variables

[Linus Walleij]

On Fri, Nov 16, 2012 at 7:50 AM, Tushar Behera <tushar.behera@linaro.org> wrote:

> Checking '< 0' for unsigned variables always returns false. For error
> codes, use IS_ERR_VALUE() instead.
>
> CC: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>

Patch applied with Viresh's ACK, thanks!
Linus Walleij

Re: [PATCH 08/14] xen: netback: Remove redundant check on unsigned variable

[Tushar Behera]

On 11/16/2012 02:46 PM, Ian Campbell wrote:
> On Fri, 2012-11-16 at 06:50 +0000, Tushar Behera wrote:
>> No need to check whether unsigned variable is less than 0.
>>
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: xen-devel@lists.xensource.com
>> CC: netdev@vger.kernel.org
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Thanks.
> 

This patch was not picked up for 3.8-rc1. Any idea, who should pick this up?

>> ---
>>  drivers/net/xen-netback/netback.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
>> index aab8677..515e10c 100644
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -190,14 +190,14 @@ static int get_page_ext(struct page *pg,
>>  
>>  	group = ext.e.group - 1;
>>  
>> -	if (group < 0 || group >= xen_netbk_group_nr)
>> +	if (group >= xen_netbk_group_nr)
>>  		return 0;
>>  
>>  	netbk = &xen_netbk[group];
>>  
>>  	idx = ext.e.idx;
>>  
>> -	if ((idx < 0) || (idx >= MAX_PENDING_REQS))
>> +	if (idx >= MAX_PENDING_REQS)
>>  		return 0;
>>  
>>  	if (netbk->mmap_pages[idx] != pg)
> 
> 


-- 
Tushar Behera

Re: [PATCH 10/14] atm: Removed redundant check on unsigned variable

[Tushar Behera]

Ping.

On 11/16/2012 12:20 PM, Tushar Behera wrote:
> No need to check whether unsigned variable is less than 0.
> 
> CC: Chas Williams <chas@cmf.nrl.navy.mil>
> CC: linux-atm-general@lists.sourceforge.net
> CC: netdev@vger.kernel.org
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/atm/fore200e.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
> index 361f5ae..fdd3fe7 100644
> --- a/drivers/atm/fore200e.c
> +++ b/drivers/atm/fore200e.c
> @@ -972,7 +972,7 @@ int bsq_audit(int where, struct host_bsq* bsq, int scheme, int magn)
>  		   where, scheme, magn, buffer->index, buffer->scheme);
>  	}
>  
> -	if ((buffer->index < 0) || (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ])) {
> +	if (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ]) {
>  	    printk(FORE200E "bsq_audit(%d): queue %d.%d, out of range buffer index = %ld !\n",
>  		   where, scheme, magn, buffer->index);
>  	}
> 


-- 
Tushar Behera

Re: [Xen-devel] [PATCH 08/14] xen: netback: Remove redundant check on unsigned variable

[Wei Liu]

On Fri, 2012-12-28 at 05:15 +0000, Tushar Behera wrote:
> On 11/16/2012 02:46 PM, Ian Campbell wrote:
> > On Fri, 2012-11-16 at 06:50 +0000, Tushar Behera wrote:
> >> No need to check whether unsigned variable is less than 0.
> >>
> >> CC: Ian Campbell <ian.campbell@citrix.com>
> >> CC: xen-devel@lists.xensource.com
> >> CC: netdev@vger.kernel.org
> >> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > Thanks.
> > 
> 
> This patch was not picked up for 3.8-rc1. Any idea, who should pick this up?

CC'ing Konrad.

Re: [Xen-devel] [PATCH 08/14] xen: netback: Remove redundant check on unsigned variable

[Wei Liu]

On Fri, 2012-12-28 at 05:15 +0000, Tushar Behera wrote:
> On 11/16/2012 02:46 PM, Ian Campbell wrote:
> > On Fri, 2012-11-16 at 06:50 +0000, Tushar Behera wrote:
> >> No need to check whether unsigned variable is less than 0.
> >>
> >> CC: Ian Campbell <ian.campbell@citrix.com>
> >> CC: xen-devel@lists.xensource.com
> >> CC: netdev@vger.kernel.org
> >> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > Thanks.
> > 
> 
> This patch was not picked up for 3.8-rc1. Any idea, who should pick this up?

CC'ing Konrad.

Re: [PATCH 10/14] atm: Removed redundant check on unsigned variable

[chas williams - CONTRACTOR]

Acked-by: chas williams - CONTRACTOR <chas@cmf.nrl.navy.mil>

On Fri, 28 Dec 2012 10:46:36 +0530
Tushar Behera <tushar.behera@linaro.org> wrote:

> Ping.
> 
> On 11/16/2012 12:20 PM, Tushar Behera wrote:
> > No need to check whether unsigned variable is less than 0.
> > 
> > CC: Chas Williams <chas@cmf.nrl.navy.mil>
> > CC: linux-atm-general@lists.sourceforge.net
> > CC: netdev@vger.kernel.org
> > Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> > ---
> >  drivers/atm/fore200e.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
> > index 361f5ae..fdd3fe7 100644
> > --- a/drivers/atm/fore200e.c
> > +++ b/drivers/atm/fore200e.c
> > @@ -972,7 +972,7 @@ int bsq_audit(int where, struct host_bsq* bsq, int scheme, int magn)
> >  		   where, scheme, magn, buffer->index, buffer->scheme);
> >  	}
> >  
> > -	if ((buffer->index < 0) || (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ])) {
> > +	if (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ]) {
> >  	    printk(FORE200E "bsq_audit(%d): queue %d.%d, out of range buffer index = %ld !\n",
> >  		   where, scheme, magn, buffer->index);
> >  	}
> > 
> 
> 

Re: [Xen-devel] [PATCH 08/14] xen: netback: Remove redundant check on unsigned variable

[Konrad Rzeszutek Wilk]

On Fri, Dec 28, 2012 at 10:41:32AM +0000, Wei Liu wrote:
> On Fri, 2012-12-28 at 05:15 +0000, Tushar Behera wrote:
> > On 11/16/2012 02:46 PM, Ian Campbell wrote:
> > > On Fri, 2012-11-16 at 06:50 +0000, Tushar Behera wrote:
> > >> No need to check whether unsigned variable is less than 0.
> > >>
> > >> CC: Ian Campbell <ian.campbell@citrix.com>
> > >> CC: xen-devel@lists.xensource.com
> > >> CC: netdev@vger.kernel.org
> > >> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> > > 
> > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > > 
> > > Thanks.
> > > 
> > 
> > This patch was not picked up for 3.8-rc1. Any idea, who should pick this up?
> 
> CC'ing Konrad.
> 

And CC-ing the network maintainer. David, Ian (who is the sub-maintainer
of xen-netback) has Acked the patch.

I can put this in my queue if you would like.

[PATCH 00/14] Modify signed comparisons of unsigned variables

[Tushar Behera]

The occurrences were identified through the coccinelle script at
following location.

http://www.emn.fr/z-info/coccinelle/rules/find_unsigned.cocci

Signed checks for unsigned variables are removed if it is also checked
for upper error limit. For error checks, IS_ERR_VALUE() macros is used.

Tushar Behera (14):
  [media] ivtv: Remove redundant check on unsigned variable
  [media] meye: Remove redundant check on unsigned variable
  [media] saa7134: Remove redundant check on unsigned variable
  [media] tlg2300: Remove redundant check on unsigned variable
  [media] atmel-isi: Update error check for unsigned variables
  pinctrl: samsung: Update error check for unsigned variables
  pinctrl: SPEAr: Update error check for unsigned variables
  xen: netback: Remove redundant check on unsigned variable
  xen: events: Remove redundant check on unsigned variable
  atm: Removed redundant check on unsigned variable
  HID: hiddev: Remove redundant check on unsigned variable
  gru: Remove redundant check on unsigned variable
  misc: tsl2550: Remove redundant check on unsigned variable
  wlcore: Remove redundant check on unsigned variable

 drivers/atm/fore200e.c                        |    2 +-
 drivers/hid/usbhid/hiddev.c                   |    2 +-
 drivers/media/pci/ivtv/ivtv-ioctl.c           |    2 +-
 drivers/media/pci/meye/meye.c                 |    2 +-
 drivers/media/pci/saa7134/saa7134-video.c     |    2 +-
 drivers/media/platform/soc_camera/atmel-isi.c |    2 +-
 drivers/media/usb/tlg2300/pd-video.c          |    2 +-
 drivers/misc/sgi-gru/grukdump.c               |    2 +-
 drivers/misc/tsl2550.c                        |    4 ++--
 drivers/net/wireless/ti/wlcore/debugfs.c      |    2 +-
 drivers/net/xen-netback/netback.c             |    4 ++--
 drivers/pinctrl/pinctrl-samsung.c             |    2 +-
 drivers/pinctrl/spear/pinctrl-plgpio.c        |    2 +-
 drivers/xen/events.c                          |    2 +-
 14 files changed, 16 insertions(+), 16 deletions(-)

-- 
1.7.4.1

CC: Mauro Carvalho Chehab <mchehab@infradead.org>
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Jeremy Fitzhardinge <jeremy@goop.org>
CC: Chas Williams <chas@cmf.nrl.navy.mil>
CC: Jack Steiner <steiner@sgi.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Luciano Coelho <coelho@ti.com>
CC: Jiri Kosina <jkosina@suse.cz>
CC: ivtv-devel@ivtvdriver.org
CC: linux-media@vger.kernel.org
CC: xen-devel@lists.xensource.com
CC: netdev@vger.kernel.org
CC: virtualization@lists.linux-foundation.org
CC: linux-atm-general@lists.sourceforge.net
CC: linux-usb@vger.kernel.org
CC: linux-input@vger.kernel.org
CC: linux-wireless@vger.kernel.org