[PATCH 01/38] arm-cci: fix handling cpumask_any_but return value

[PATCH 01/38] arm-cci: fix handling cpumask_any_but return value

[Andrzej Hajda]

cpumask_any_but returns value >= nr_cpu_ids if there are no more CPUs.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/bus/arm-cci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 577cc4b..22754d0 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -1314,7 +1314,7 @@ static int cci_pmu_cpu_notifier(struct notifier_block *self,
 		if (!cpumask_test_and_clear_cpu(cpu, &cci_pmu->cpus))
 			break;
 		target = cpumask_any_but(cpu_online_mask, cpu);
-		if (target < 0) // UP, last CPU
+		if (target >= nr_cpu_ids) // UP, last CPU
 			break;
 		/*
 		 * TODO: migrate context once core races on event->ctx have
-- 
1.9.1

Re: [PATCH 01/38] arm-cci: fix handling cpumask_any_but return value

[Will Deacon]

On Mon, Sep 21, 2015 at 02:33:33PM +0100, Andrzej Hajda wrote:
> cpumask_any_but returns value >= nr_cpu_ids if there are no more CPUs.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/bus/arm-cci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 577cc4b..22754d0 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -1314,7 +1314,7 @@ static int cci_pmu_cpu_notifier(struct notifier_block *self,
>  		if (!cpumask_test_and_clear_cpu(cpu, &cci_pmu->cpus))
>  			break;
>  		target = cpumask_any_but(cpu_online_mask, cpu);
> -		if (target < 0) // UP, last CPU
> +		if (target >= nr_cpu_ids) // UP, last CPU

We could also make target an int, to match the return type of
cpumask_any_but (on top of this change).

Regardless,

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH 01/38] arm-cci: fix handling cpumask_any_but return value

[Andrzej Hajda]

Hi,

Ping - almost three months passed.

Regards
Andrzej

On 09/21/2015 03:33 PM, Andrzej Hajda wrote:
> cpumask_any_but returns value >= nr_cpu_ids if there are no more CPUs.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/bus/arm-cci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 577cc4b..22754d0 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -1314,7 +1314,7 @@ static int cci_pmu_cpu_notifier(struct notifier_block *self,
>  		if (!cpumask_test_and_clear_cpu(cpu, &cci_pmu->cpus))
>  			break;
>  		target = cpumask_any_but(cpu_online_mask, cpu);
> -		if (target < 0) // UP, last CPU
> +		if (target >= nr_cpu_ids) // UP, last CPU
>  			break;
>  		/*
>  		 * TODO: migrate context once core races on event->ctx have

Re: [PATCH 01/38] arm-cci: fix handling cpumask_any_but return value

[Suzuki K. Poulose]

On 14/12/15 10:20, Andrzej Hajda wrote:
> Hi,
>
> Ping - almost three months passed.

Andrzej,

The cci patches normally go via the arm-soc tree (which is not quit obvious).
Unfortunately, your post didn't have the maintainers alias - arm@vger.kernel.org -
added (except for Arnd).

Arnd,

What do you recommend ? Should Adrzej send the patch again (with arm@vger.kernel.org in
Cc) or could you pick this one up ?

Regards
Suzuki


>
> Regards
> Andrzej
>
> On 09/21/2015 03:33 PM, Andrzej Hajda wrote:
>> cpumask_any_but returns value >= nr_cpu_ids if there are no more CPUs.
>>
>> The problem has been detected using proposed semantic patch
>> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>>
>> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>   drivers/bus/arm-cci.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
>> index 577cc4b..22754d0 100644
>> --- a/drivers/bus/arm-cci.c
>> +++ b/drivers/bus/arm-cci.c
>> @@ -1314,7 +1314,7 @@ static int cci_pmu_cpu_notifier(struct notifier_block *self,
>>              if (!cpumask_test_and_clear_cpu(cpu, &cci_pmu->cpus))
>>                      break;
>>              target = cpumask_any_but(cpu_online_mask, cpu);
>> -            if (target < 0) // UP, last CPU
>> +            if (target >= nr_cpu_ids) // UP, last CPU
>>                      break;
>>              /*
>>               * TODO: migrate context once core races on event->ctx have
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


________________________________

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 02/38] bus: arm-ccn: fix handling cpumask_any_but return value

[Andrzej Hajda]

cpumask_any_but returns value >= nr_cpu_ids if there are no more CPUs.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/bus/arm-ccn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
index 7d9879e..cc322fb 100644
--- a/drivers/bus/arm-ccn.c
+++ b/drivers/bus/arm-ccn.c
@@ -1184,7 +1184,7 @@ static int arm_ccn_pmu_cpu_notifier(struct notifier_block *nb,
 		if (!cpumask_test_and_clear_cpu(cpu, &dt->cpu))
 			break;
 		target = cpumask_any_but(cpu_online_mask, cpu);
-		if (target < 0)
+		if (target >= nr_cpu_ids)
 			break;
 		perf_pmu_migrate_context(&dt->pmu, cpu, target);
 		cpumask_set_cpu(target, &dt->cpu);
-- 
1.9.1

Re: [PATCH 02/38] bus: arm-ccn: fix handling cpumask_any_but return value

[Pawel Moll]

Dnia 2015-09-21, Mon o godzinie 15:33 +0200, Andrzej Hajda pisze:
> cpumask_any_but returns value >= nr_cpu_ids if there are no more CPUs.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

This has been already reported and discussed:

	http://thread.gmane.org/gmane.linux.kernel.janitors/34058

but apparently slipped through cracks :-( therefore:

Acked-by: Pawel Moll <pawel.moll@arm.com>

Thanks for bringing it up again!

Pawel

[PATCH 03/38] drm/i915: fix handling gen8_emit_flush_coherentl3_wa result

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fe06accb0..ff9a481 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1254,9 +1254,10 @@ static int gen8_init_indirectctx_bb(struct intel_engine_cs *ring,
 
 	/* WaFlushCoherentL3CacheLinesAtContextSwitch:bdw */
 	if (IS_BROADWELL(ring->dev)) {
-		index = gen8_emit_flush_coherentl3_wa(ring, batch, index);
-		if (index < 0)
-			return index;
+		int rc = gen8_emit_flush_coherentl3_wa(ring, batch, index);
+		if (rc < 0)
+			return rc;
+		index = rc;
 	}
 
 	/* WaClearSlmSpaceAtContextSwitch:bdw,chv */
-- 
1.9.1

Re: [PATCH 03/38] drm/i915: fix handling gen8_emit_flush_coherentl3_wa result

[Jani Nikula]

On Mon, 21 Sep 2015, Andrzej Hajda <a.hajda@samsung.com> wrote:
> The function can return negative value.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index fe06accb0..ff9a481 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1254,9 +1254,10 @@ static int gen8_init_indirectctx_bb(struct intel_engine_cs *ring,
>  
>  	/* WaFlushCoherentL3CacheLinesAtContextSwitch:bdw */
>  	if (IS_BROADWELL(ring->dev)) {
> -		index = gen8_emit_flush_coherentl3_wa(ring, batch, index);
> -		if (index < 0)
> -			return index;
> +		int rc = gen8_emit_flush_coherentl3_wa(ring, batch, index);
> +		if (rc < 0)
> +			return rc;
> +		index = rc;
>  	}
>  
>  	/* WaClearSlmSpaceAtContextSwitch:bdw,chv */
> -- 
> 1.9.1
>

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 03/38] drm/i915: fix handling gen8_emit_flush_coherentl3_wa result

[Daniel Vetter]

On Mon, Sep 21, 2015 at 04:59:58PM +0300, Jani Nikula wrote:
> On Mon, 21 Sep 2015, Andrzej Hajda <a.hajda@samsung.com> wrote:
> > The function can return negative value.
> >
> > The problem has been detected using proposed semantic patch
> > scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> >
> > [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> >
> > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 04/38] IB/ehca: fix handling idr_alloc result

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/staging/rdma/ehca/ehca_cq.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rdma/ehca/ehca_cq.c b/drivers/staging/rdma/ehca/ehca_cq.c
index 9b68b17..ea1b5c1 100644
--- a/drivers/staging/rdma/ehca/ehca_cq.c
+++ b/drivers/staging/rdma/ehca/ehca_cq.c
@@ -130,7 +130,7 @@ struct ib_cq *ehca_create_cq(struct ib_device *device,
 	void *vpage;
 	u32 counter;
 	u64 rpage, cqx_fec, h_ret;
-	int ipz_rc, i;
+	int rc, i;
 	unsigned long flags;
 
 	if (attr->flags)
@@ -170,16 +170,17 @@ struct ib_cq *ehca_create_cq(struct ib_device *device,
 
 	idr_preload(GFP_KERNEL);
 	write_lock_irqsave(&ehca_cq_idr_lock, flags);
-	my_cq->token = idr_alloc(&ehca_cq_idr, my_cq, 0, 0x2000000, GFP_NOWAIT);
+	rc = idr_alloc(&ehca_cq_idr, my_cq, 0, 0x2000000, GFP_NOWAIT);
 	write_unlock_irqrestore(&ehca_cq_idr_lock, flags);
 	idr_preload_end();
 
-	if (my_cq->token < 0) {
+	if (rc < 0) {
 		cq = ERR_PTR(-ENOMEM);
 		ehca_err(device, "Can't allocate new idr entry. device=%p",
 			 device);
 		goto create_cq_exit1;
 	}
+	my_cq->token = rc;
 
 	/*
 	 * CQs maximum depth is 4GB-64, but we need additional 20 as buffer
@@ -195,11 +196,11 @@ struct ib_cq *ehca_create_cq(struct ib_device *device,
 		goto create_cq_exit2;
 	}
 
-	ipz_rc = ipz_queue_ctor(NULL, &my_cq->ipz_queue, param.act_pages,
+	rc = ipz_queue_ctor(NULL, &my_cq->ipz_queue, param.act_pages,
 				EHCA_PAGESIZE, sizeof(struct ehca_cqe), 0, 0);
-	if (!ipz_rc) {
+	if (!rc) {
 		ehca_err(device, "ipz_queue_ctor() failed ipz_rc=%i device=%p",
-			 ipz_rc, device);
+			 rc, device);
 		cq = ERR_PTR(-EINVAL);
 		goto create_cq_exit3;
 	}
-- 
1.9.1

Re: [PATCH 04/38] IB/ehca: fix handling idr_alloc result

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 2311 bytes --]

On 09/21/2015 09:33 AM, Andrzej Hajda wrote:
> The function can return negative value.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Patch looks fine to me.

Acked-by: Doug Ledford <dledford@redhat.com>

> ---
>  drivers/staging/rdma/ehca/ehca_cq.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/rdma/ehca/ehca_cq.c b/drivers/staging/rdma/ehca/ehca_cq.c
> index 9b68b17..ea1b5c1 100644
> --- a/drivers/staging/rdma/ehca/ehca_cq.c
> +++ b/drivers/staging/rdma/ehca/ehca_cq.c
> @@ -130,7 +130,7 @@ struct ib_cq *ehca_create_cq(struct ib_device *device,
>  	void *vpage;
>  	u32 counter;
>  	u64 rpage, cqx_fec, h_ret;
> -	int ipz_rc, i;
> +	int rc, i;
>  	unsigned long flags;
>  
>  	if (attr->flags)
> @@ -170,16 +170,17 @@ struct ib_cq *ehca_create_cq(struct ib_device *device,
>  
>  	idr_preload(GFP_KERNEL);
>  	write_lock_irqsave(&ehca_cq_idr_lock, flags);
> -	my_cq->token = idr_alloc(&ehca_cq_idr, my_cq, 0, 0x2000000, GFP_NOWAIT);
> +	rc = idr_alloc(&ehca_cq_idr, my_cq, 0, 0x2000000, GFP_NOWAIT);
>  	write_unlock_irqrestore(&ehca_cq_idr_lock, flags);
>  	idr_preload_end();
>  
> -	if (my_cq->token < 0) {
> +	if (rc < 0) {
>  		cq = ERR_PTR(-ENOMEM);
>  		ehca_err(device, "Can't allocate new idr entry. device=%p",
>  			 device);
>  		goto create_cq_exit1;
>  	}
> +	my_cq->token = rc;
>  
>  	/*
>  	 * CQs maximum depth is 4GB-64, but we need additional 20 as buffer
> @@ -195,11 +196,11 @@ struct ib_cq *ehca_create_cq(struct ib_device *device,
>  		goto create_cq_exit2;
>  	}
>  
> -	ipz_rc = ipz_queue_ctor(NULL, &my_cq->ipz_queue, param.act_pages,
> +	rc = ipz_queue_ctor(NULL, &my_cq->ipz_queue, param.act_pages,
>  				EHCA_PAGESIZE, sizeof(struct ehca_cqe), 0, 0);
> -	if (!ipz_rc) {
> +	if (!rc) {
>  		ehca_err(device, "ipz_queue_ctor() failed ipz_rc=%i device=%p",
> -			 ipz_rc, device);
> +			 rc, device);
>  		cq = ERR_PTR(-EINVAL);
>  		goto create_cq_exit3;
>  	}
> 


-- 
Doug Ledford <dledford@redhat.com>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

[PATCH 05/38] staging: lustre: fix handling lustre_posix_acl_xattr_filter result

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/staging/lustre/lustre/llite/xattr.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
index 362a87d..dddc1ff 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -175,11 +175,12 @@ int ll_setxattr_common(struct inode *inode, const char *name,
 			}
 			ee_free(ee);
 		} else if (rce->rce_ops == RMT_RSETFACL) {
-			size = lustre_posix_acl_xattr_filter(
+			rc = lustre_posix_acl_xattr_filter(
 						(posix_acl_xattr_header *)value,
 						size, &new_value);
-			if (unlikely(size < 0))
-				return size;
+			if (unlikely(rc < 0))
+				return rc;
+			size = rc;
 
 			pv = (const char *)new_value;
 		} else
-- 
1.9.1

[PATCH 06/38] tty: serial: lpc32xx_hs: fix handling platform_get_irq result

[Andrzej Hajda]

The function can return negative values.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/tty/serial/lpc32xx_hs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/lpc32xx_hs.c b/drivers/tty/serial/lpc32xx_hs.c
index e92d7eb..7eb04ae 100644
--- a/drivers/tty/serial/lpc32xx_hs.c
+++ b/drivers/tty/serial/lpc32xx_hs.c
@@ -691,12 +691,13 @@ static int serial_hs_lpc32xx_probe(struct platform_device *pdev)
 	p->port.mapbase = res->start;
 	p->port.membase = NULL;
 
-	p->port.irq = platform_get_irq(pdev, 0);
-	if (p->port.irq < 0) {
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0) {
 		dev_err(&pdev->dev, "Error getting irq for HS UART port %d\n",
 			uarts_registered);
-		return p->port.irq;
+		return ret;
 	}
+	p->port.irq = ret;
 
 	p->port.iotype = UPIO_MEM32;
 	p->port.uartclk = LPC32XX_MAIN_OSC_FREQ;
-- 
1.9.1

[PATCH 07/38] usb: host: ehci-msm: fix handling platform_get_irq result

[Andrzej Hajda]

The function can return negative values.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/usb/host/ehci-msm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index 275c92e..c4f84c8 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -80,12 +80,12 @@ static int ehci_msm_probe(struct platform_device *pdev)
 		return  -ENOMEM;
 	}
 
-	hcd->irq = platform_get_irq(pdev, 0);
-	if (hcd->irq < 0) {
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0) {
 		dev_err(&pdev->dev, "Unable to get IRQ resource\n");
-		ret = hcd->irq;
 		goto put_hcd;
 	}
+	hcd->irq = ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
-- 
1.9.1

Re: [PATCH 07/38] usb: host: ehci-msm: fix handling platform_get_irq result

[Alan Stern]

On Mon, 21 Sep 2015, Andrzej Hajda wrote:

> The function can return negative values.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/usb/host/ehci-msm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
> index 275c92e..c4f84c8 100644
> --- a/drivers/usb/host/ehci-msm.c
> +++ b/drivers/usb/host/ehci-msm.c
> @@ -80,12 +80,12 @@ static int ehci_msm_probe(struct platform_device *pdev)
>  		return  -ENOMEM;
>  	}
>  
> -	hcd->irq = platform_get_irq(pdev, 0);
> -	if (hcd->irq < 0) {
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0) {
>  		dev_err(&pdev->dev, "Unable to get IRQ resource\n");
> -		ret = hcd->irq;
>  		goto put_hcd;
>  	}
> +	hcd->irq = ret;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {

Acked-by: Alan Stern <stern@rowland.harvard.edu>

You might consider extending the coccinelle script to look for even
worse problems.  ehci-mv.c doesn't test for negative return values at
all!

Alan Stern

[PATCH 08/38] openvswitch: fix handling result of ipv6_skip_exthdr

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 net/openvswitch/conntrack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 002a755..fde3391 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -253,7 +253,7 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto)
 	const struct nf_conntrack_helper *helper;
 	const struct nf_conn_help *help;
 	enum ip_conntrack_info ctinfo;
-	unsigned int protoff;
+	int protoff;
 	struct nf_conn *ct;
 
 	ct = nf_ct_get(skb, &ctinfo);
-- 
1.9.1

Re: [PATCH 08/38] openvswitch: fix handling result of ipv6_skip_exthdr

[Pravin Shelar]

On Mon, Sep 21, 2015 at 6:33 AM, Andrzej Hajda <a.hajda@samsung.com> wrote:
> The function can return negative value.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  net/openvswitch/conntrack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 002a755..fde3391 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -253,7 +253,7 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto)
>         const struct nf_conntrack_helper *helper;
>         const struct nf_conn_help *help;
>         enum ip_conntrack_info ctinfo;
> -       unsigned int protoff;
> +       int protoff;
>         struct nf_conn *ct;
>

A patch is already pushed to the net tree to fix this issue.

[PATCH 09/38] selftests/timers: fix write return value handlng

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 tools/testing/selftests/timers/clocksource-switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c
index 627ec74..fd88e30 100644
--- a/tools/testing/selftests/timers/clocksource-switch.c
+++ b/tools/testing/selftests/timers/clocksource-switch.c
@@ -97,7 +97,7 @@ int get_cur_clocksource(char *buf, size_t size)
 int change_clocksource(char *clocksource)
 {
 	int fd;
-	size_t size;
+	ssize_t size;
 
 	fd = open("/sys/devices/system/clocksource/clocksource0/current_clocksource", O_WRONLY);
 
-- 
1.9.1

Re: [PATCH 09/38] selftests/timers: fix write return value handlng

[John Stultz]

On Mon, Sep 21, 2015 at 6:33 AM, Andrzej Hajda <a.hajda@samsung.com> wrote:
> The function can return negative value.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Sheesh. Apologies for being so late on this. I just noticed I
miss-filed this and it slipped through my to-queue list.

Looks good. Thanks for sending this in. I'll queue it for 4.5

thanks
-john

[PATCH 10/38] hwrng: fix handling platform_get_irq

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/char/hw_random/xgene-rng.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/char/hw_random/xgene-rng.c b/drivers/char/hw_random/xgene-rng.c
index c37cf75..3c77645 100644
--- a/drivers/char/hw_random/xgene-rng.c
+++ b/drivers/char/hw_random/xgene-rng.c
@@ -344,11 +344,12 @@ static int xgene_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(ctx->csr_base))
 		return PTR_ERR(ctx->csr_base);
 
-	ctx->irq = platform_get_irq(pdev, 0);
-	if (ctx->irq < 0) {
+	rc = platform_get_irq(pdev, 0);
+	if (rc < 0) {
 		dev_err(&pdev->dev, "No IRQ resource\n");
-		return ctx->irq;
+		return rc;
 	}
+	ctx->irq = rc;
 
 	dev_dbg(&pdev->dev, "APM X-Gene RNG BASE %p ALARM IRQ %d",
 		ctx->csr_base, ctx->irq);
-- 
1.9.1

Re: [PATCH 10/38] hwrng: fix handling platform_get_irq

[Herbert Xu]

On Mon, Sep 21, 2015 at 03:33:42PM +0200, Andrzej Hajda wrote:
> The function can return negative value.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied to crypto.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 11/38] HSI: omap_ssi: fix handling ida_simple_get result

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/hsi/controllers/omap_ssi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/hsi/controllers/omap_ssi.c b/drivers/hsi/controllers/omap_ssi.c
index 089c6c3..5d9225f 100644
--- a/drivers/hsi/controllers/omap_ssi.c
+++ b/drivers/hsi/controllers/omap_ssi.c
@@ -336,11 +336,10 @@ static int __init ssi_add_controller(struct hsi_controller *ssi,
 		return -ENOMEM;
 	}
 
-	ssi->id = ida_simple_get(&platform_omap_ssi_ida, 0, 0, GFP_KERNEL);
-	if (ssi->id < 0) {
-		err = ssi->id;
+	err = ida_simple_get(&platform_omap_ssi_ida, 0, 0, GFP_KERNEL);
+	if (err < 0)
 		goto out_err;
-	}
+	ssi->id = err;
 
 	ssi->owner = THIS_MODULE;
 	ssi->device.parent = &pd->dev;
-- 
1.9.1

Re: [PATCH 11/38] HSI: omap_ssi: fix handling ida_simple_get result

[Andrzej Hajda]

Hi,

Ping - almost three months passed.

Regards
Andrzej


On 09/21/2015 03:33 PM, Andrzej Hajda wrote:
> The function can return negative value.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/hsi/controllers/omap_ssi.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hsi/controllers/omap_ssi.c b/drivers/hsi/controllers/omap_ssi.c
> index 089c6c3..5d9225f 100644
> --- a/drivers/hsi/controllers/omap_ssi.c
> +++ b/drivers/hsi/controllers/omap_ssi.c
> @@ -336,11 +336,10 @@ static int __init ssi_add_controller(struct hsi_controller *ssi,
>  		return -ENOMEM;
>  	}
>  
> -	ssi->id = ida_simple_get(&platform_omap_ssi_ida, 0, 0, GFP_KERNEL);
> -	if (ssi->id < 0) {
> -		err = ssi->id;
> +	err = ida_simple_get(&platform_omap_ssi_ida, 0, 0, GFP_KERNEL);
> +	if (err < 0)
>  		goto out_err;
> -	}
> +	ssi->id = err;
>  
>  	ssi->owner = THIS_MODULE;
>  	ssi->device.parent = &pd->dev;

Re: [PATCH 11/38] HSI: omap_ssi: fix handling ida_simple_get result

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

Hi,

On Mon, Dec 14, 2015 at 11:27:11AM +0100, Andrzej Hajda wrote:
> On 09/21/2015 03:33 PM, Andrzej Hajda wrote:
> > The function can return negative value.
> >
> > The problem has been detected using proposed semantic patch
> > scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> >
> > [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> >
> > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>
> Ping - almost three months passed.

Sorry, I missed those two patches. The fixes are not very complex,
so I queued them for 4.5.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 12/38] HSI: omap_ssi_port: fix handling of_get_named_gpio result

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/hsi/controllers/omap_ssi_port.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hsi/controllers/omap_ssi_port.c b/drivers/hsi/controllers/omap_ssi_port.c
index 1f8652b..d93dd09 100644
--- a/drivers/hsi/controllers/omap_ssi_port.c
+++ b/drivers/hsi/controllers/omap_ssi_port.c
@@ -1147,13 +1147,13 @@ static int __init ssi_port_probe(struct platform_device *pd)
 		goto error;
 	}
 
-	cawake_gpio = of_get_named_gpio(np, "ti,ssi-cawake-gpio", 0);
-	if (cawake_gpio < 0) {
+	err = of_get_named_gpio(np, "ti,ssi-cawake-gpio", 0);
+	if (err < 0) {
 		dev_err(&pd->dev, "DT data is missing cawake gpio (err=%d)\n",
-			cawake_gpio);
-		err = -ENODEV;
+			err);
 		goto error;
 	}
+	cawake_gpio = err;
 
 	err = devm_gpio_request_one(&port->device, cawake_gpio, GPIOF_DIR_IN,
 		"cawake");
-- 
1.9.1

Re: [PATCH 12/38] HSI: omap_ssi_port: fix handling of_get_named_gpio result

[Andrzej Hajda]

Hi,

Ping - almost three months passed.

Regards
Andrzej


On 09/21/2015 03:33 PM, Andrzej Hajda wrote:
> The function can return negative value.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/hsi/controllers/omap_ssi_port.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hsi/controllers/omap_ssi_port.c b/drivers/hsi/controllers/omap_ssi_port.c
> index 1f8652b..d93dd09 100644
> --- a/drivers/hsi/controllers/omap_ssi_port.c
> +++ b/drivers/hsi/controllers/omap_ssi_port.c
> @@ -1147,13 +1147,13 @@ static int __init ssi_port_probe(struct platform_device *pd)
>  		goto error;
>  	}
>  
> -	cawake_gpio = of_get_named_gpio(np, "ti,ssi-cawake-gpio", 0);
> -	if (cawake_gpio < 0) {
> +	err = of_get_named_gpio(np, "ti,ssi-cawake-gpio", 0);
> +	if (err < 0) {
>  		dev_err(&pd->dev, "DT data is missing cawake gpio (err=%d)\n",
> -			cawake_gpio);
> -		err = -ENODEV;
> +			err);
>  		goto error;
>  	}
> +	cawake_gpio = err;
>  
>  	err = devm_gpio_request_one(&port->device, cawake_gpio, GPIOF_DIR_IN,
>  		"cawake");

[PATCH 13/38] ARM: shmobile: apmu: correct type of CPU id

[Andrzej Hajda]

CPU id can be negative, so it cannot be assigned to unsigned variable.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/arm/mach-shmobile/platsmp-apmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/platsmp-apmu.c b/arch/arm/mach-shmobile/platsmp-apmu.c
index 4e54512..292c67b 100644
--- a/arch/arm/mach-shmobile/platsmp-apmu.c
+++ b/arch/arm/mach-shmobile/platsmp-apmu.c
@@ -88,7 +88,7 @@ static void apmu_init_cpu(struct resource *res, int cpu, int bit)
 static void apmu_parse_cfg(void (*fn)(struct resource *res, int cpu, int bit),
 			   struct rcar_apmu_config *apmu_config, int num)
 {
-	u32 id;
+	int id;
 	int k;
 	int bit, index;
 	bool is_allowed;
-- 
1.9.1

Re: [PATCH 13/38] ARM: shmobile: apmu: correct type of CPU id

[Simon Horman]

On Mon, Sep 21, 2015 at 03:33:45PM +0200, Andrzej Hajda wrote:
> CPU id can be negative, so it cannot be assigned to unsigned variable.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Thanks, I have queued this up as a cleanup for v4.4.

[PATCH 14/38] clk: vt8500: fix sign of possible PLL values

[Andrzej Hajda]

With unsigned values underflow in loops can occur resulting in
theoretically infinite loops.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/clk/clk-vt8500.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c
index 37e9288..098e9fa 100644
--- a/drivers/clk/clk-vt8500.c
+++ b/drivers/clk/clk-vt8500.c
@@ -384,7 +384,7 @@ static void vt8500_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 static void wm8650_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 				u32 *multiplier, u32 *divisor1, u32 *divisor2)
 {
-	u32 mul, div1, div2;
+	int mul, div1, div2;
 	u32 best_mul, best_div1, best_div2;
 	unsigned long tclk, rate_err, best_err;
 
@@ -452,7 +452,7 @@ static u32 wm8750_get_filter(u32 parent_rate, u32 divisor1)
 static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 				u32 *filter, u32 *multiplier, u32 *divisor1, u32 *divisor2)
 {
-	u32 mul, div1, div2;
+	int mul, div1, div2;
 	u32 best_mul, best_div1, best_div2;
 	unsigned long tclk, rate_err, best_err;
 
@@ -496,7 +496,7 @@ static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 static void wm8850_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 				u32 *multiplier, u32 *divisor1, u32 *divisor2)
 {
-	u32 mul, div1, div2;
+	int mul, div1, div2;
 	u32 best_mul, best_div1, best_div2;
 	unsigned long tclk, rate_err, best_err;
 
-- 
1.9.1

Re: [PATCH 14/38] clk: vt8500: fix sign of possible PLL values

[Stephen Boyd]

On 09/21, Andrzej Hajda wrote:
> With unsigned values underflow in loops can occur resulting in
> theoretically infinite loops.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/clk/clk-vt8500.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c
> index 37e9288..098e9fa 100644
> --- a/drivers/clk/clk-vt8500.c
> +++ b/drivers/clk/clk-vt8500.c
> @@ -384,7 +384,7 @@ static void vt8500_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  static void wm8650_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	int mul, div1, div2;

Only div2 seems to need the treatment here.

>  	u32 best_mul, best_div1, best_div2;
>  	unsigned long tclk, rate_err, best_err;
>  
> @@ -452,7 +452,7 @@ static u32 wm8750_get_filter(u32 parent_rate, u32 divisor1)
>  static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *filter, u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	int mul, div1, div2;

And here div1 and div2 are the only ones.

>  	u32 best_mul, best_div1, best_div2;
>  	unsigned long tclk, rate_err, best_err;
>  
> @@ -496,7 +496,7 @@ static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  static void wm8850_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	int mul, div1, div2;

And div1 and div2 here.

Can we focus the changes on the actual problematic variables? I
suppose this has never mattered because we break out of these
loops.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 14/38] clk: vt8500: fix sign of possible PLL values

[Andrzej Hajda]

With unsigned values underflow in loops can occur resulting in
theoretically infinite loops.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi Stephen,

This is modified version according to your request, ie only problematic
variables have changed type.
I still think that 1st version is better, but of course it is up to you.

Regards
Andrzej

 drivers/clk/clk-vt8500.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c
index 37e9288..98c4492 100644
--- a/drivers/clk/clk-vt8500.c
+++ b/drivers/clk/clk-vt8500.c
@@ -384,7 +384,8 @@ static void vt8500_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 static void wm8650_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 				u32 *multiplier, u32 *divisor1, u32 *divisor2)
 {
-	u32 mul, div1, div2;
+	u32 mul, div1;
+	int div2;
 	u32 best_mul, best_div1, best_div2;
 	unsigned long tclk, rate_err, best_err;
 
@@ -452,7 +453,8 @@ static u32 wm8750_get_filter(u32 parent_rate, u32 divisor1)
 static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 				u32 *filter, u32 *multiplier, u32 *divisor1, u32 *divisor2)
 {
-	u32 mul, div1, div2;
+	u32 mul;
+	int div1, div2;
 	u32 best_mul, best_div1, best_div2;
 	unsigned long tclk, rate_err, best_err;
 
@@ -496,7 +498,8 @@ static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 static void wm8850_find_pll_bits(unsigned long rate, unsigned long parent_rate,
 				u32 *multiplier, u32 *divisor1, u32 *divisor2)
 {
-	u32 mul, div1, div2;
+	u32 mul;
+	int div1, div2;
 	u32 best_mul, best_div1, best_div2;
 	unsigned long tclk, rate_err, best_err;
 
-- 
1.9.1

Re: [PATCH v2 14/38] clk: vt8500: fix sign of possible PLL values

[Andrzej Hajda]

Hi,

Ping.

Regards
Andrzej

On 10/02/2015 06:49 AM, Andrzej Hajda wrote:

> With unsigned values underflow in loops can occur resulting in
> theoretically infinite loops.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi Stephen,
>
> This is modified version according to your request, ie only problematic
> variables have changed type.
> I still think that 1st version is better, but of course it is up to you.
>
> Regards
> Andrzej
>
>  drivers/clk/clk-vt8500.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c
> index 37e9288..98c4492 100644
> --- a/drivers/clk/clk-vt8500.c
> +++ b/drivers/clk/clk-vt8500.c
> @@ -384,7 +384,8 @@ static void vt8500_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  static void wm8650_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	u32 mul, div1;
> +	int div2;
>  	u32 best_mul, best_div1, best_div2;
>  	unsigned long tclk, rate_err, best_err;
>  
> @@ -452,7 +453,8 @@ static u32 wm8750_get_filter(u32 parent_rate, u32 divisor1)
>  static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *filter, u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	u32 mul;
> +	int div1, div2;
>  	u32 best_mul, best_div1, best_div2;
>  	unsigned long tclk, rate_err, best_err;
>  
> @@ -496,7 +498,8 @@ static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  static void wm8850_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	u32 mul;
> +	int div1, div2;
>  	u32 best_mul, best_div1, best_div2;
>  	unsigned long tclk, rate_err, best_err;
>  

Re: [PATCH v2 14/38] clk: vt8500: fix sign of possible PLL values

[Andrzej Hajda]

Ping

Regards
Andrzej

On 10/02/2015 06:49 AM, Andrzej Hajda wrote:
> With unsigned values underflow in loops can occur resulting in
> theoretically infinite loops.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi Stephen,
>
> This is modified version according to your request, ie only problematic
> variables have changed type.
> I still think that 1st version is better, but of course it is up to you.
>
> Regards
> Andrzej
>
>  drivers/clk/clk-vt8500.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c
> index 37e9288..98c4492 100644
> --- a/drivers/clk/clk-vt8500.c
> +++ b/drivers/clk/clk-vt8500.c
> @@ -384,7 +384,8 @@ static void vt8500_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  static void wm8650_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	u32 mul, div1;
> +	int div2;
>  	u32 best_mul, best_div1, best_div2;
>  	unsigned long tclk, rate_err, best_err;
>  
> @@ -452,7 +453,8 @@ static u32 wm8750_get_filter(u32 parent_rate, u32 divisor1)
>  static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *filter, u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	u32 mul;
> +	int div1, div2;
>  	u32 best_mul, best_div1, best_div2;
>  	unsigned long tclk, rate_err, best_err;
>  
> @@ -496,7 +498,8 @@ static void wm8750_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  static void wm8850_find_pll_bits(unsigned long rate, unsigned long parent_rate,
>  				u32 *multiplier, u32 *divisor1, u32 *divisor2)
>  {
> -	u32 mul, div1, div2;
> +	u32 mul;
> +	int div1, div2;
>  	u32 best_mul, best_div1, best_div2;
>  	unsigned long tclk, rate_err, best_err;
>  

Re: [PATCH v2 14/38] clk: vt8500: fix sign of possible PLL values

[Stephen Boyd]

On 10/02, Andrzej Hajda wrote:
> With unsigned values underflow in loops can occur resulting in
> theoretically infinite loops.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 15/38] drm/layerscape: fix handling fsl_dcu_drm_plane_index result

[Andrzej Hajda]

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 82be6b8..d1e300d 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -58,7 +58,8 @@ static void fsl_dcu_drm_plane_atomic_disable(struct drm_plane *plane,
 					     struct drm_plane_state *old_state)
 {
 	struct fsl_dcu_drm_device *fsl_dev = plane->dev->dev_private;
-	unsigned int index, value, ret;
+	unsigned int value;
+	int index, ret;
 
 	index = fsl_dcu_drm_plane_index(plane);
 	if (index < 0)
-- 
1.9.1

[PATCH 16/38] gpu: ipu-v3: fix div_ratio type

[Andrzej Hajda]

The variable can be negative.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/ipu-v3/ipu-csi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index 752cdd2..b6c3a82 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -202,7 +202,7 @@ static int ipu_csi_set_testgen_mclk(struct ipu_csi *csi, u32 pixel_clk,
 					u32 ipu_clk)
 {
 	u32 temp;
-	u32 div_ratio;
+	int div_ratio;
 
 	div_ratio = (ipu_clk / pixel_clk) - 1;
 
-- 
1.9.1

Re: [PATCH 16/38] gpu: ipu-v3: fix div_ratio type

[Philipp Zabel]

Hi Andrzej,

Am Montag, den 21.09.2015, 15:33 +0200 schrieb Andrzej Hajda:
> The variable can be negative.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied, thank you.

regards
Philipp

[PATCH 17/38] isdn: hisax: fix frame calculation

[Andrzej Hajda]

Difference of unsigned values is also unsigned so it does not make
sense to check its sign.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/isdn/hisax/hfc4s8s_l1.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/isdn/hisax/hfc4s8s_l1.c b/drivers/isdn/hisax/hfc4s8s_l1.c
index 0e5d673..9600cd7 100644
--- a/drivers/isdn/hisax/hfc4s8s_l1.c
+++ b/drivers/isdn/hisax/hfc4s8s_l1.c
@@ -646,14 +646,14 @@ rx_d_frame(struct hfc4s8s_l1 *l1p, int ech)
 
 		f1 = Read_hfc8_stable(l1p->hw, A_F1);
 		f2 = Read_hfc8(l1p->hw, A_F2);
-		df = f1 - f2;
-		if ((f1 - f2) < 0)
-			df = f1 - f2 + MAX_F_CNT + 1;
 
+		if (f1 < f2)
+			df = MAX_F_CNT + 1 + f1 - f2;
+		else
+			df = f1 - f2;
 
-		if (!df) {
+		if (!df)
 			return;	/* no complete frame in fifo */
-		}
 
 		z1 = Read_hfc16_stable(l1p->hw, A_Z1);
 		z2 = Read_hfc16(l1p->hw, A_Z2);
-- 
1.9.1

Re: [PATCH 17/38] isdn: hisax: fix frame calculation

[David Miller]

From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:49 +0200

> Difference of unsigned values is also unsigned so it does not make
> sense to check its sign.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied.

[PATCH 18/38] net/ibm/emac: fix type of phy_mode

[Andrzej Hajda]

phy_mode can be negative.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/net/ethernet/ibm/emac/core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.h b/drivers/net/ethernet/ibm/emac/core.h
index 28df374..f379e47 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -181,7 +181,7 @@ struct emac_instance {
 	struct mal_commac		commac;
 
 	/* PHY infos */
-	u32				phy_mode;
+	int				phy_mode;
 	u32				phy_map;
 	u32				phy_address;
 	u32				phy_feat_exc;
-- 
1.9.1

Re: [PATCH 18/38] net/ibm/emac: fix type of phy_mode

[David Miller]

From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:50 +0200

> phy_mode can be negative.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied.

[PATCH 19/38] net: stmmac: fix type of entry variable

[Andrzej Hajda]

Variable can store negative values.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 925f2f8..934143e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1945,7 +1945,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
 	unsigned int txsize = priv->dma_tx_size;
-	unsigned int entry;
+	int entry;
 	int i, csum_insertion = 0, is_jumbo = 0;
 	int nfrags = skb_shinfo(skb)->nr_frags;
 	struct dma_desc *desc, *first;
-- 
1.9.1

Re: [PATCH 19/38] net: stmmac: fix type of entry variable

[David Miller]

From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:51 +0200

> Variable can store negative values.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied.

[PATCH 20/38] net: brcm80211: fix range check

[Andrzej Hajda]

Unsigned minus constant is still unsigned so checking its sign makes no
sense.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/net/wireless/brcm80211/brcmsmac/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index 9728be0..218cbc8 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -4585,7 +4585,7 @@ static int brcms_b_attach(struct brcms_c_info *wlc, struct bcma_device *core,
 		wlc_hw->machwcap_backup = wlc_hw->machwcap;
 
 		/* init tx fifo size */
-		WARN_ON((wlc_hw->corerev - XMTFIFOTBL_STARTREV) < 0 ||
+		WARN_ON(wlc_hw->corerev < XMTFIFOTBL_STARTREV ||
 			(wlc_hw->corerev - XMTFIFOTBL_STARTREV) >
 				ARRAY_SIZE(xmtfifo_sz));
 		wlc_hw->xmtfifo_sz =
-- 
1.9.1

Re: [PATCH 20/38] net: brcm80211: fix range check

[David Miller]

From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:52 +0200

> Unsigned minus constant is still unsigned so checking its sign makes no
> sense.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied.

[PATCH 21/38] mwifiex: fix comparison expression

[Andrzej Hajda]

To avoid underflows signed variables should be used in expression.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/net/wireless/mwifiex/11n_rxreorder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/11n_rxreorder.c b/drivers/net/wireless/mwifiex/11n_rxreorder.c
index 2906cd5..b3970a8 100644
--- a/drivers/net/wireless/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/mwifiex/11n_rxreorder.c
@@ -615,10 +615,10 @@ int mwifiex_11n_rx_reorder_pkt(struct mwifiex_private *priv,
 	    ((end_win > start_win) && ((seq_num > end_win) ||
 				       (seq_num < start_win)))) {
 		end_win = seq_num;
-		if (((seq_num - win_size) + 1) >= 0)
+		if (((end_win - win_size) + 1) >= 0)
 			start_win = (end_win - win_size) + 1;
 		else
-			start_win = (MAX_TID_VALUE - (win_size - seq_num)) + 1;
+			start_win = (MAX_TID_VALUE - (win_size - end_win)) + 1;
 		mwifiex_11n_dispatch_pkt_until_start_win(priv, tbl, start_win);
 	}
 
-- 
1.9.1

RE: [PATCH 21/38] mwifiex: fix comparison expression

[Amitkumar Karwar]

Hi Andrzej,

> From: Andrzej Hajda [mailto:a.hajda@samsung.com]
> Sent: Monday, September 21, 2015 7:04 PM
> To: linux-kernel@vger.kernel.org
> Cc: Andrzej Hajda; Bartlomiej Zolnierkiewicz; Marek Szyprowski;
> Amitkumar Karwar; Nishant Sarmukadam; Kalle Valo; linux-
> wireless@vger.kernel.org; netdev@vger.kernel.org
> Subject: [PATCH 21/38] mwifiex: fix comparison expression
> 
> To avoid underflows signed variables should be used in expression.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/net/wireless/mwifiex/11n_rxreorder.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/mwifiex/11n_rxreorder.c
> b/drivers/net/wireless/mwifiex/11n_rxreorder.c
> index 2906cd5..b3970a8 100644
> --- a/drivers/net/wireless/mwifiex/11n_rxreorder.c
> +++ b/drivers/net/wireless/mwifiex/11n_rxreorder.c
> @@ -615,10 +615,10 @@ int mwifiex_11n_rx_reorder_pkt(struct
> mwifiex_private *priv,
>  	    ((end_win > start_win) && ((seq_num > end_win) ||
>  				       (seq_num < start_win)))) {
>  		end_win = seq_num;
> -		if (((seq_num - win_size) + 1) >= 0)
> +		if (((end_win - win_size) + 1) >= 0)
>  			start_win = (end_win - win_size) + 1;
>  		else
> -			start_win = (MAX_TID_VALUE - (win_size - seq_num)) +
> 1;
> +			start_win = (MAX_TID_VALUE - (win_size - end_win)) +
> 1;
>  		mwifiex_11n_dispatch_pkt_until_start_win(priv, tbl,
> start_win);
>  	}
> 

Acked-by: Amitkumar Karwar <akarwar@marvell.com>

Regards,
Amitkumar

Re: [PATCH 21/38] mwifiex: fix comparison expression

[David Miller]

From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:53 +0200

> To avoid underflows signed variables should be used in expression.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied.

[PATCH 22/38] orinoco: fix checking for default value

[Andrzej Hajda]

Thresholds uses -1 to indicate that default value should be used.
Since thresholds are unsigned sign checking makes no sense.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/net/wireless/orinoco/cfg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/orinoco/cfg.c b/drivers/net/wireless/orinoco/cfg.c
index a9e94b6..0f6ea31 100644
--- a/drivers/net/wireless/orinoco/cfg.c
+++ b/drivers/net/wireless/orinoco/cfg.c
@@ -220,7 +220,7 @@ static int orinoco_set_wiphy_params(struct wiphy *wiphy, u32 changed)
 	if (changed & WIPHY_PARAM_FRAG_THRESHOLD) {
 		/* Set fragmentation */
 		if (priv->has_mwo) {
-			if (wiphy->frag_threshold < 0)
+			if (wiphy->frag_threshold == -1)
 				frag_value = 0;
 			else {
 				printk(KERN_WARNING "%s: Fixed fragmentation "
@@ -230,7 +230,7 @@ static int orinoco_set_wiphy_params(struct wiphy *wiphy, u32 changed)
 				frag_value = 1;
 			}
 		} else {
-			if (wiphy->frag_threshold < 0)
+			if (wiphy->frag_threshold == -1)
 				frag_value = 2346;
 			else if ((wiphy->frag_threshold < 257) ||
 				 (wiphy->frag_threshold > 2347))
@@ -252,7 +252,7 @@ static int orinoco_set_wiphy_params(struct wiphy *wiphy, u32 changed)
 		 * the upper limit.
 		 */
 
-		if (wiphy->rts_threshold < 0)
+		if (wiphy->rts_threshold == -1)
 			rts_value = 2347;
 		else if (wiphy->rts_threshold > 2347)
 			err = -EINVAL;
-- 
1.9.1

Re: [PATCH 22/38] orinoco: fix checking for default value

[David Miller]

From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:54 +0200

> Thresholds uses -1 to indicate that default value should be used.
> Since thresholds are unsigned sign checking makes no sense.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied.

[PATCH 23/38] rndis_wlan: fix checking for default value

[Andrzej Hajda]

Thresholds uses -1 to indicate that default value should be used.
Since thresholds are unsigned sign checking makes no sense.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/net/wireless/rndis_wlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 71a825c..a13d1f2 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1236,7 +1236,7 @@ static int set_rts_threshold(struct usbnet *usbdev, u32 rts_threshold)
 
 	netdev_dbg(usbdev->net, "%s(): %i\n", __func__, rts_threshold);
 
-	if (rts_threshold < 0 || rts_threshold > 2347)
+	if (rts_threshold == -1 || rts_threshold > 2347)
 		rts_threshold = 2347;
 
 	tmp = cpu_to_le32(rts_threshold);
-- 
1.9.1

Re: [PATCH 23/38] rndis_wlan: fix checking for default value

[David Miller]

From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:55 +0200

> Thresholds uses -1 to indicate that default value should be used.
> Since thresholds are unsigned sign checking makes no sense.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied.

[PATCH 24/38] rtc: opal: fix type of token

[Andrzej Hajda]

The variable can take signed values.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/rtc/rtc-opal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-opal.c b/drivers/rtc/rtc-opal.c
index 6fbf9e6..590e1a4 100644
--- a/drivers/rtc/rtc-opal.c
+++ b/drivers/rtc/rtc-opal.c
@@ -152,10 +152,10 @@ exit:
 /* Set Timed Power-On */
 static int opal_set_tpo_time(struct device *dev, struct rtc_wkalrm *alarm)
 {
-	u64 h_m_s_ms = 0, token;
+	u64 h_m_s_ms = 0;
 	struct opal_msg msg;
 	u32 y_m_d = 0;
-	int rc;
+	int token, rc;
 
 	tm_to_opal(&alarm->time, &y_m_d, &h_m_s_ms);
 
-- 
1.9.1

Re: [PATCH 24/38] rtc: opal: fix type of token

[Alexandre Belloni]

On 21/09/2015 at 15:33:56 +0200, Andrzej Hajda wrote :
> The variable can take signed values.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/rtc/rtc-opal.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 25/38] staging: media: davinci_vpfe: fix ipipe_mode type

[Andrzej Hajda]

The variable can take negative values.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
index 2a3a56b..b1d5e23 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
@@ -254,7 +254,7 @@ int config_ipipe_hw(struct vpfe_ipipe_device *ipipe)
 	void __iomem *ipipe_base = ipipe->base_addr;
 	struct v4l2_mbus_framefmt *outformat;
 	u32 color_pat;
-	u32 ipipe_mode;
+	int ipipe_mode;
 	u32 data_path;
 
 	/* enable clock to IPIPE */
-- 
1.9.1

Re: [PATCH 25/38] staging: media: davinci_vpfe: fix ipipe_mode type

[Laurent Pinchart]

Hi Andrzej,

Thank you for the patch.

On Monday 21 September 2015 15:33:57 Andrzej Hajda wrote:
> The variable can take negative values.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and applied to my tree.

> ---
>  drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c index
> 2a3a56b..b1d5e23 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> @@ -254,7 +254,7 @@ int config_ipipe_hw(struct vpfe_ipipe_device *ipipe)
>  	void __iomem *ipipe_base = ipipe->base_addr;
>  	struct v4l2_mbus_framefmt *outformat;
>  	u32 color_pat;
> -	u32 ipipe_mode;
> +	int ipipe_mode;
>  	u32 data_path;
> 
>  	/* enable clock to IPIPE */

-- 
Regards,

Laurent Pinchart

[PATCH 26/38] staging: lustre: remove invalid check

[Andrzej Hajda]

Unsigned cannot be negative.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/staging/lustre/lustre/osc/lproc_osc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/osc/lproc_osc.c b/drivers/staging/lustre/lustre/osc/lproc_osc.c
index c504d15..f3eb39f 100644
--- a/drivers/staging/lustre/lustre/osc/lproc_osc.c
+++ b/drivers/staging/lustre/lustre/osc/lproc_osc.c
@@ -451,9 +451,6 @@ static ssize_t resend_count_store(struct kobject *kobj,
 	if (rc)
 		return rc;
 
-	if (val < 0)
-	       return -EINVAL;
-
 	atomic_set(&obd->u.cli.cl_resends, val);
 
 	return count;
-- 
1.9.1

[PATCH 27/38] usbnet: remove invalid check

[Andrzej Hajda]

skb->len is always non-negative.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/net/usb/lan78xx.c  | 5 -----
 drivers/net/usb/smsc75xx.c | 5 -----
 drivers/net/usb/smsc95xx.c | 5 -----
 3 files changed, 15 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index a39518f..e0556dc 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2522,11 +2522,6 @@ static int lan78xx_rx(struct lan78xx_net *dev, struct sk_buff *skb)
 			skb_pull(skb, align_count);
 	}
 
-	if (unlikely(skb->len < 0)) {
-		netdev_warn(dev->net, "invalid rx length<0 %d", skb->len);
-		return 0;
-	}
-
 	return 1;
 }
 
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index d9e7892..30033db 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2185,11 +2185,6 @@ static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 			skb_pull(skb, align_count);
 	}
 
-	if (unlikely(skb->len < 0)) {
-		netdev_warn(dev->net, "invalid rx length<0 %d\n", skb->len);
-		return 0;
-	}
-
 	return 1;
 }
 
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 26423ad..66b3ab9 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1815,11 +1815,6 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 			skb_pull(skb, align_count);
 	}
 
-	if (unlikely(skb->len < 0)) {
-		netdev_warn(dev->net, "invalid rx length<0 %d\n", skb->len);
-		return 0;
-	}
-
 	return 1;
 }
 
-- 
1.9.1

Re: [PATCH 27/38] usbnet: remove invalid check

[David Miller]

From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:59 +0200

> skb->len is always non-negative.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied.

[PATCH 28/38] video/omap: remove invalid check

[Andrzej Hajda]

regno is unsigned so it cannot be negative.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/video/fbdev/omap/omapfb_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/video/fbdev/omap/omapfb_main.c b/drivers/video/fbdev/omap/omapfb_main.c
index 1fb3ea3..393ae1b 100644
--- a/drivers/video/fbdev/omap/omapfb_main.c
+++ b/drivers/video/fbdev/omap/omapfb_main.c
@@ -276,11 +276,6 @@ static int _setcolreg(struct fb_info *info, u_int regno, u_int red, u_int green,
 		if (r != 0)
 			break;
 
-		if (regno < 0) {
-			r = -EINVAL;
-			break;
-		}
-
 		if (regno < 16) {
 			u16 pal;
 			pal = ((red >> (16 - var->red.length)) <<
-- 
1.9.1

Re: [PATCH 28/38] video/omap: remove invalid check

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]



On 21/09/15 16:34, Andrzej Hajda wrote:
> regno is unsigned so it cannot be negative.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/video/fbdev/omap/omapfb_main.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/video/fbdev/omap/omapfb_main.c b/drivers/video/fbdev/omap/omapfb_main.c
> index 1fb3ea3..393ae1b 100644
> --- a/drivers/video/fbdev/omap/omapfb_main.c
> +++ b/drivers/video/fbdev/omap/omapfb_main.c
> @@ -276,11 +276,6 @@ static int _setcolreg(struct fb_info *info, u_int regno, u_int red, u_int green,
>  		if (r != 0)
>  			break;
>  
> -		if (regno < 0) {
> -			r = -EINVAL;
> -			break;
> -		}
> -
>  		if (regno < 16) {
>  			u16 pal;
>  			pal = ((red >> (16 - var->red.length)) <<
> 

Thanks, queued for 4.4.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 29/38] Input: touchscreen: atmel: remove invalid check

[Andrzej Hajda]

byte_offset is unsigned.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index c562205..c577f95 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1267,7 +1267,7 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data,
 
 			byte_offset = reg + i - cfg_start_ofs;
 
-			if (byte_offset >= 0 && byte_offset < config_mem_size) {
+			if (byte_offset < config_mem_size) {
 				*(config_mem + byte_offset) = val;
 			} else {
 				dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
-- 
1.9.1

Re: [PATCH 29/38] Input: touchscreen: atmel: remove invalid check

[Dmitry Torokhov]

Hi Andrzej,

On Mon, Sep 21, 2015 at 03:34:01PM +0200, Andrzej Hajda wrote:
> byte_offset is unsigned.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index c562205..c577f95 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1267,7 +1267,7 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data,
>  
>  			byte_offset = reg + i - cfg_start_ofs;
>  
> -			if (byte_offset >= 0 && byte_offset < config_mem_size) {
> +			if (byte_offset < config_mem_size) {

I'd rather we kept the check as is: it documents the expected range even
if one of the checks is always true. If you search the archives you'll
find Linus has also pretty strong opinion about these checks actually
being useful from documenting POV.


>  				*(config_mem + byte_offset) = val;
>  			} else {
>  				dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
> -- 
> 1.9.1
> 

Thanks.

-- 
Dmitry

[PATCH 30/38] leds: flash: remove invalid check

[Andrzej Hajda]

Unsigned cannot be lesser than zero.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/leds/led-class-flash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 3b25734..c209077 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -108,7 +108,7 @@ static ssize_t flash_strobe_store(struct device *dev,
 	if (ret)
 		goto unlock;
 
-	if (state < 0 || state > 1) {
+	if (state > 1) {
 		ret = -EINVAL;
 		goto unlock;
 	}
-- 
1.9.1

Re: [PATCH 30/38] leds: flash: remove invalid check

[Jacek Anaszewski]

Hi Andrzej,

On 09/21/2015 03:34 PM, Andrzej Hajda wrote:
> Unsigned cannot be lesser than zero.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>   drivers/leds/led-class-flash.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
> index 3b25734..c209077 100644
> --- a/drivers/leds/led-class-flash.c
> +++ b/drivers/leds/led-class-flash.c
> @@ -108,7 +108,7 @@ static ssize_t flash_strobe_store(struct device *dev,
>   	if (ret)
>   		goto unlock;
>
> -	if (state < 0 || state > 1) {
> +	if (state > 1) {
>   		ret = -EINVAL;
>   		goto unlock;
>   	}
>

Merged, thanks.

-- 
Best Regards,
Jacek Anaszewski

[PATCH 31/38] leds: tca6507: remove invalid check

[Andrzej Hajda]

unsigned value cannot be lesser than zero.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/leds/leds-tca6507.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
index edbecc4..93564f6 100644
--- a/drivers/leds/leds-tca6507.c
+++ b/drivers/leds/leds-tca6507.c
@@ -713,7 +713,7 @@ tca6507_led_dt_init(struct i2c_client *client)
 		if (of_property_match_string(child, "compatible", "gpio") >= 0)
 			led.flags |= TCA6507_MAKE_GPIO;
 		ret = of_property_read_u32(child, "reg", &reg);
-		if (ret != 0 || reg < 0 || reg >= NUM_LEDS)
+		if (ret != 0 || reg >= NUM_LEDS)
 			continue;
 
 		tca_leds[reg] = led;
-- 
1.9.1

Re: [PATCH 31/38] leds: tca6507: remove invalid check

[Jacek Anaszewski]

Hi Andrzej,

On 09/21/2015 03:34 PM, Andrzej Hajda wrote:
> unsigned value cannot be lesser than zero.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>   drivers/leds/leds-tca6507.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
> index edbecc4..93564f6 100644
> --- a/drivers/leds/leds-tca6507.c
> +++ b/drivers/leds/leds-tca6507.c
> @@ -713,7 +713,7 @@ tca6507_led_dt_init(struct i2c_client *client)
>   		if (of_property_match_string(child, "compatible", "gpio") >= 0)
>   			led.flags |= TCA6507_MAKE_GPIO;
>   		ret = of_property_read_u32(child, "reg", &reg);
> -		if (ret != 0 || reg < 0 || reg >= NUM_LEDS)
> +		if (ret != 0 || reg >= NUM_LEDS)
>   			continue;
>
>   		tca_leds[reg] = led;
>

Merged, thanks.

-- 
Best Regards,
Jacek Anaszewski

[PATCH 32/38] fs/cachefiles: remove invalid checks

[Andrzej Hajda]

Unsigned values cannot be lesser than zero.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 fs/cachefiles/bind.c   | 9 +++------
 fs/cachefiles/daemon.c | 6 +++---
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/cachefiles/bind.c b/fs/cachefiles/bind.c
index 6af790f..6b50c88 100644
--- a/fs/cachefiles/bind.c
+++ b/fs/cachefiles/bind.c
@@ -39,13 +39,10 @@ int cachefiles_daemon_bind(struct cachefiles_cache *cache, char *args)
 	       args);
 
 	/* start by checking things over */
-	ASSERT(cache->fstop_percent >= 0 &&
-	       cache->fstop_percent < cache->fcull_percent &&
-	       cache->fcull_percent < cache->frun_percent &&
-	       cache->frun_percent  < 100);
+	ASSERT(cache->fstop_percent < cache->fcull_percent &&
+	       cache->fcull_percent < cache->frun_percent);
 
-	ASSERT(cache->bstop_percent >= 0 &&
-	       cache->bstop_percent < cache->bcull_percent &&
+	ASSERT(cache->bstop_percent < cache->bcull_percent &&
 	       cache->bcull_percent < cache->brun_percent &&
 	       cache->brun_percent  < 100);
 
diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index f601def..996ecd2 100644
--- a/fs/cachefiles/daemon.c
+++ b/fs/cachefiles/daemon.c
@@ -222,7 +222,7 @@ static ssize_t cachefiles_daemon_write(struct file *file,
 	if (test_bit(CACHEFILES_DEAD, &cache->flags))
 		return -EIO;
 
-	if (datalen < 0 || datalen > PAGE_SIZE - 1)
+	if (datalen > PAGE_SIZE - 1)
 		return -EOPNOTSUPP;
 
 	/* drag the command string into the kernel so we can parse it */
@@ -385,7 +385,7 @@ static int cachefiles_daemon_fstop(struct cachefiles_cache *cache, char *args)
 	if (args[0] != '%' || args[1] != '\0')
 		return -EINVAL;
 
-	if (fstop < 0 || fstop >= cache->fcull_percent)
+	if (fstop >= cache->fcull_percent)
 		return cachefiles_daemon_range_error(cache, args);
 
 	cache->fstop_percent = fstop;
@@ -457,7 +457,7 @@ static int cachefiles_daemon_bstop(struct cachefiles_cache *cache, char *args)
 	if (args[0] != '%' || args[1] != '\0')
 		return -EINVAL;
 
-	if (bstop < 0 || bstop >= cache->bcull_percent)
+	if (bstop >= cache->bcull_percent)
 		return cachefiles_daemon_range_error(cache, args);
 
 	cache->bstop_percent = bstop;
-- 
1.9.1

[PATCH 33/38] mm/memblock.c: remove invalid check

[Andrzej Hajda]

Unsigned value cannot be lesser than zero.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 mm/memblock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index d300f13..aeb5148 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -837,7 +837,7 @@ void __init_memblock __next_reserved_mem_region(u64 *idx,
 {
 	struct memblock_type *type = &memblock.reserved;
 
-	if (*idx >= 0 && *idx < type->cnt) {
+	if (*idx < type->cnt) {
 		struct memblock_region *r = &type->regions[*idx];
 		phys_addr_t base = r->base;
 		phys_addr_t size = r->size;
-- 
1.9.1

Re: [PATCH 33/38] mm/memblock.c: remove invalid check

[Andrew Morton]

On Mon, 21 Sep 2015 15:34:05 +0200 Andrzej Hajda <a.hajda@samsung.com> wrote:

> Unsigned value cannot be lesser than zero.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> ...
>
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -837,7 +837,7 @@ void __init_memblock __next_reserved_mem_region(u64 *idx,
>  {
>  	struct memblock_type *type = &memblock.reserved;
>  
> -	if (*idx >= 0 && *idx < type->cnt) {
> +	if (*idx < type->cnt) {

Linus has in the past expressed a preference for retaining checks such
as this.  iirc he finds it clearer.  And perhaps safer if the type
should change in the future.

[PATCH 34/38] perf: remove invalid check

[Andrzej Hajda]

Unsigned values cannot be lesser than zero.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/sh/kernel/ptrace_32.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c
index c1a6b89..6125ce9 100644
--- a/arch/sh/kernel/ptrace_32.c
+++ b/arch/sh/kernel/ptrace_32.c
@@ -419,8 +419,7 @@ long arch_ptrace(struct task_struct *child, long request,
 
 	case PTRACE_POKEUSR: /* write the word at location addr in the USER area */
 		ret = -EIO;
-		if ((addr & 3) || addr < 0 ||
-		    addr > sizeof(struct user) - 3)
+		if ((addr & 3) || addr > sizeof(struct user) - 3)
 			break;
 
 		if (addr < sizeof(struct pt_regs))
-- 
1.9.1

[PATCH 35/38] ptrace: remove invalid check

[Andrzej Hajda]

Unsigned values cannot be lesser than zero.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/sh/kernel/ptrace_64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sh/kernel/ptrace_64.c b/arch/sh/kernel/ptrace_64.c
index 5cea973..138b2b1 100644
--- a/arch/sh/kernel/ptrace_64.c
+++ b/arch/sh/kernel/ptrace_64.c
@@ -395,7 +395,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		unsigned long tmp;
 
 		ret = -EIO;
-		if ((addr & 3) || addr < 0)
+		if (addr & 3)
 			break;
 
 		if (addr < sizeof(struct pt_regs))
@@ -423,7 +423,7 @@ long arch_ptrace(struct task_struct *child, long request,
                    this could crash the kernel or result in a security
                    loophole. */
 		ret = -EIO;
-		if ((addr & 3) || addr < 0)
+		if (addr & 3)
 			break;
 
 		if (addr < sizeof(struct pt_regs)) {
-- 
1.9.1

[PATCH 36/38] MIPS: remove invalid check

[Andrzej Hajda]

Unsigned values cannot be lesser than zero.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/mips/mm/sc-mips.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
index 53ea839..1755187 100644
--- a/arch/mips/mm/sc-mips.c
+++ b/arch/mips/mm/sc-mips.c
@@ -162,13 +162,13 @@ static inline int __init mips_sc_probe(void)
 		return 0;
 
 	tmp = (config2 >> 8) & 0x0f;
-	if (0 <= tmp && tmp <= 7)
+	if (tmp <= 7)
 		c->scache.sets = 64 << tmp;
 	else
 		return 0;
 
 	tmp = (config2 >> 0) & 0x0f;
-	if (0 <= tmp && tmp <= 7)
+	if (tmp <= 7)
 		c->scache.ways = tmp + 1;
 	else
 		return 0;
-- 
1.9.1

Re: [PATCH 36/38] MIPS: remove invalid check

[Ralf Baechle]

On Mon, Sep 21, 2015 at 03:34:08PM +0200, Andrzej Hajda wrote:

> Unsigned values cannot be lesser than zero.
> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Chris Dearman's original commit 9318c51acd9689505850152cc98277a6d6f2d752
([MIPS] MIPS32/MIPS64 secondary cache management) introduced these less
than zero checks in 2.6.18.  They're fortunately entirely harmless.

Patch queued for 4.4.  Thanks!

  Ralf

[PATCH 37/38] zlib_deflate/deftree: change always true condition to 1

[Andrzej Hajda]

Unsigned value is always greater of equal zero.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 lib/zlib_deflate/deftree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/zlib_deflate/deftree.c b/lib/zlib_deflate/deftree.c
index 9b1756b..8bd736e 100644
--- a/lib/zlib_deflate/deftree.c
+++ b/lib/zlib_deflate/deftree.c
@@ -935,7 +935,7 @@ ulg zlib_tr_flush_block(
         zlib_tr_stored_block(s, buf, stored_len, eof);
 
 #ifdef FORCE_STATIC
-    } else if (static_lenb >= 0) { /* force static trees */
+    } else if (1) { /* force static trees */
 #else
     } else if (static_lenb == opt_lenb) {
 #endif
-- 
1.9.1

[PATCH 38/38] drm/radeon: simplify boot level calculation

[Andrzej Hajda]

The patch simplifies the code without changing behaviour, but most
problably there is a bug somewhere else.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/amd/amdgpu/kv_dpm.c | 11 +----------
 drivers/gpu/drm/radeon/kv_dpm.c     | 11 +----------
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
index 94ec04a..f9cfc56 100644
--- a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
@@ -1622,19 +1622,10 @@ static int kv_update_samu_dpm(struct amdgpu_device *adev, bool gate)
 
 static u8 kv_get_acp_boot_level(struct amdgpu_device *adev)
 {
-	u8 i;
 	struct amdgpu_clock_voltage_dependency_table *table =
 		&adev->pm.dpm.dyn_state.acp_clock_voltage_dependency_table;
 
-	for (i = 0; i < table->count; i++) {
-		if (table->entries[i].clk >= 0) /* XXX */
-			break;
-	}
-
-	if (i >= table->count)
-		i = table->count - 1;
-
-	return i;
+	return table->count ? 0 : -1;
 }
 
 static void kv_update_acp_boot_level(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/radeon/kv_dpm.c b/drivers/gpu/drm/radeon/kv_dpm.c
index 2d71da4..dc9aab5 100644
--- a/drivers/gpu/drm/radeon/kv_dpm.c
+++ b/drivers/gpu/drm/radeon/kv_dpm.c
@@ -1546,19 +1546,10 @@ static int kv_update_samu_dpm(struct radeon_device *rdev, bool gate)
 
 static u8 kv_get_acp_boot_level(struct radeon_device *rdev)
 {
-	u8 i;
 	struct radeon_clock_voltage_dependency_table *table =
 		&rdev->pm.dpm.dyn_state.acp_clock_voltage_dependency_table;
 
-	for (i = 0; i < table->count; i++) {
-		if (table->entries[i].clk >= 0) /* XXX */
-			break;
-	}
-
-	if (i >= table->count)
-		i = table->count - 1;
-
-	return i;
+	return table->count ? 0 : -1;
 }
 
 static void kv_update_acp_boot_level(struct radeon_device *rdev)
-- 
1.9.1

RE: [PATCH 38/38] drm/radeon: simplify boot level calculation

[Deucher, Alexander]

> -----Original Message-----
> From: Andrzej Hajda [mailto:a.hajda@samsung.com]
> Sent: Monday, September 21, 2015 9:34 AM
> To: linux-kernel@vger.kernel.org
> Cc: Andrzej Hajda; Bartlomiej Zolnierkiewicz; Marek Szyprowski; David Airlie;
> Deucher, Alexander; Koenig, Christian; dri-devel@lists.freedesktop.org
> Subject: [PATCH 38/38] drm/radeon: simplify boot level calculation
> 
> The patch simplifies the code without changing behaviour, but most
> problably there is a bug somewhere else.

I'd prefer the leave the code as is in case we ever add ACP support to these asics.

Thanks,

Alex

> 
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
> 
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/kv_dpm.c | 11 +----------
>  drivers/gpu/drm/radeon/kv_dpm.c     | 11 +----------
>  2 files changed, 2 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
> b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
> index 94ec04a..f9cfc56 100644
> --- a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c
> @@ -1622,19 +1622,10 @@ static int kv_update_samu_dpm(struct
> amdgpu_device *adev, bool gate)
> 
>  static u8 kv_get_acp_boot_level(struct amdgpu_device *adev)
>  {
> -	u8 i;
>  	struct amdgpu_clock_voltage_dependency_table *table =
>  		&adev-
> >pm.dpm.dyn_state.acp_clock_voltage_dependency_table;
> 
> -	for (i = 0; i < table->count; i++) {
> -		if (table->entries[i].clk >= 0) /* XXX */
> -			break;
> -	}
> -
> -	if (i >= table->count)
> -		i = table->count - 1;
> -
> -	return i;
> +	return table->count ? 0 : -1;
>  }
> 
>  static void kv_update_acp_boot_level(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/radeon/kv_dpm.c
> b/drivers/gpu/drm/radeon/kv_dpm.c
> index 2d71da4..dc9aab5 100644
> --- a/drivers/gpu/drm/radeon/kv_dpm.c
> +++ b/drivers/gpu/drm/radeon/kv_dpm.c
> @@ -1546,19 +1546,10 @@ static int kv_update_samu_dpm(struct
> radeon_device *rdev, bool gate)
> 
>  static u8 kv_get_acp_boot_level(struct radeon_device *rdev)
>  {
> -	u8 i;
>  	struct radeon_clock_voltage_dependency_table *table =
>  		&rdev-
> >pm.dpm.dyn_state.acp_clock_voltage_dependency_table;
> 
> -	for (i = 0; i < table->count; i++) {
> -		if (table->entries[i].clk >= 0) /* XXX */
> -			break;
> -	}
> -
> -	if (i >= table->count)
> -		i = table->count - 1;
> -
> -	return i;
> +	return table->count ? 0 : -1;
>  }
> 
>  static void kv_update_acp_boot_level(struct radeon_device *rdev)
> --
> 1.9.1

Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types

[David Howells]

Andrzej Hajda <a.hajda@samsung.com> wrote:

> Semantic patch finds comparisons of types:
>     unsigned < 0
>     unsigned >= 0
> The former is always false, the latter is always true.
> Such comparisons are useless, so theoretically they could be
> safely removed, but their presence quite often indicates bugs.

Or someone has left them in because they don't matter and there's the
possibility that the type being tested might be or become signed under some
circumstances.  If the comparison is useless, I'd expect the compiler to just
discard it - for such cases your patch is pointless.

If I have, for example:

	unsigned x;

	if (x == 0 || x > 27)
		give_a_range_error();

I will write this as:

	unsigned x;

	if (x <= 0 || x > 27)
		give_a_range_error();

because it that gives a way to handle x being changed to signed at some point
in the future for no cost.  In which case, your changing the <= to an ==
"because the < part of the case is useless" is arguably wrong.

David

Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types

[Andrzej Hajda]

On 09/21/2015 03:42 PM, David Howells wrote:
> Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> 
>> Semantic patch finds comparisons of types:
>>     unsigned < 0
>>     unsigned >= 0
>> The former is always false, the latter is always true.
>> Such comparisons are useless, so theoretically they could be
>> safely removed, but their presence quite often indicates bugs.
> 
> Or someone has left them in because they don't matter and there's the
> possibility that the type being tested might be or become signed under some
> circumstances.  If the comparison is useless, I'd expect the compiler to just
> discard it - for such cases your patch is pointless.
> 
> If I have, for example:
> 
> 	unsigned x;
> 
> 	if (x == 0 || x > 27)
> 		give_a_range_error();
> 
> I will write this as:
> 
> 	unsigned x;
> 
> 	if (x <= 0 || x > 27)
> 		give_a_range_error();
> 
> because it that gives a way to handle x being changed to signed at some point
> in the future for no cost.  In which case, your changing the <= to an ==
> "because the < part of the case is useless" is arguably wrong.

This is why I have not checked for such cases - I have skipped checks of type
	unsigned <= 0
exactly for the reasons above.

However I have left two other checks as they seems to me more suspicious - they
are always true or false. But as Dmitry and Andrew pointed out Linus have quite
strong opinion against removing range checks in such cases as he finds it
clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37.

Regards
Andrzej

> 
> David
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types

[Jacek Anaszewski]

On 09/22/2015 11:13 AM, Andrzej Hajda wrote:
> On 09/21/2015 03:42 PM, David Howells wrote:
>> Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>
>>> Semantic patch finds comparisons of types:
>>>      unsigned < 0
>>>      unsigned >= 0
>>> The former is always false, the latter is always true.
>>> Such comparisons are useless, so theoretically they could be
>>> safely removed, but their presence quite often indicates bugs.
>>
>> Or someone has left them in because they don't matter and there's the
>> possibility that the type being tested might be or become signed under some
>> circumstances.  If the comparison is useless, I'd expect the compiler to just
>> discard it - for such cases your patch is pointless.
>>
>> If I have, for example:
>>
>> 	unsigned x;
>>
>> 	if (x == 0 || x > 27)
>> 		give_a_range_error();
>>
>> I will write this as:
>>
>> 	unsigned x;
>>
>> 	if (x <= 0 || x > 27)
>> 		give_a_range_error();
>>
>> because it that gives a way to handle x being changed to signed at some point
>> in the future for no cost.  In which case, your changing the <= to an ==
>> "because the < part of the case is useless" is arguably wrong.
>
> This is why I have not checked for such cases - I have skipped checks of type
> 	unsigned <= 0
> exactly for the reasons above.
>
> However I have left two other checks as they seems to me more suspicious - they
> are always true or false. But as Dmitry and Andrew pointed out Linus have quite
> strong opinion against removing range checks in such cases as he finds it
> clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37.

Dropped 30/38 and 31/38 from LED tree then.

-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH 32/38] fs/cachefiles: remove invalid checks

[David Howells]

Andrzej Hajda <a.hajda@samsung.com> wrote:

> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

This is a problem in your test.  There's no actual problem with the code.

> -	ASSERT(cache->fstop_percent >= 0 &&

This is fine.  The compiler should just ignore it.

> -	       cache->fstop_percent < cache->fcull_percent &&
> -	       cache->fcull_percent < cache->frun_percent &&
> -	       cache->frun_percent  < 100);
> +	ASSERT(cache->fstop_percent < cache->fcull_percent &&
> +	       cache->fcull_percent < cache->frun_percent);

You've lost the upper bound check.

> -	if (datalen < 0 || datalen > PAGE_SIZE - 1)
> -	if (fstop < 0 || fstop >= cache->fcull_percent)
> -	if (bstop < 0 || bstop >= cache->bcull_percent)

These are all fine.  The compiler should just ignore them.

David

Re: [PATCH 32/38] fs/cachefiles: remove invalid checks

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> > -	ASSERT(cache->fstop_percent >= 0 &&
> 
> This is fine.  The compiler should just ignore it.

By fine, I mean fine as-is.  It shouldn't need changing.

David