spi: atmel: Fix test unsigned var < 0
diff mbox

Message ID 1395498809.16958.1.camel@phoenix
State New, archived
Headers show

Commit Message

Axel Lin March 22, 2014, 2:33 p.m. UTC
current_remaining_bytes is an unsigned long and cannot be below 0.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/spi/spi-atmel.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Mark Brown March 26, 2014, 4:51 p.m. UTC | #1
On Sat, Mar 22, 2014 at 10:33:29PM +0800, Axel Lin wrote:
> current_remaining_bytes is an unsigned long and cannot be below 0.

> -	if (xfer->bits_per_word > 8) {
> +
> +	if (xfer->bits_per_word > 8)
>  		as->current_remaining_bytes -= 2;
> -		if (as->current_remaining_bytes < 0)
> -			as->current_remaining_bytes = 0;
> -	} else {
> +	else
>  		as->current_remaining_bytes--;
> -	}

This removes an error check in the case that we set the remaining bytes
to -1.  The length validation the core does should ensure that never
happens but it seems wrong to just ignore that - we should at least note
in the changelog that the analysis has been done.

Are you sure that the best fix isn't to just use an int here?
Axel Lin March 27, 2014, 12:46 a.m. UTC | #2
2014-03-27 0:51 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Sat, Mar 22, 2014 at 10:33:29PM +0800, Axel Lin wrote:
>> current_remaining_bytes is an unsigned long and cannot be below 0.
>
>> -     if (xfer->bits_per_word > 8) {
>> +
>> +     if (xfer->bits_per_word > 8)
>>               as->current_remaining_bytes -= 2;
>> -             if (as->current_remaining_bytes < 0)
>> -                     as->current_remaining_bytes = 0;
>> -     } else {
>> +     else
>>               as->current_remaining_bytes--;
>> -     }
>
> This removes an error check in the case that we set the remaining bytes
> to -1.  The length validation the core does should ensure that never
> happens but it seems wrong to just ignore that - we should at least note
> in the changelog that the analysis has been done.
I thought it never happen and there is no bug report for this so it's
safe with this patch.

>
> Are you sure that the best fix isn't to just use an int here?
Wenyou,
  Any comments for this?

Regards,
Axel
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index bda961e..3e8c4cc 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -873,13 +873,11 @@  atmel_spi_pump_pio_data(struct atmel_spi *as, struct spi_transfer *xfer)
 	} else {
 		spi_readl(as, RDR);
 	}
-	if (xfer->bits_per_word > 8) {
+
+	if (xfer->bits_per_word > 8)
 		as->current_remaining_bytes -= 2;
-		if (as->current_remaining_bytes < 0)
-			as->current_remaining_bytes = 0;
-	} else {
+	else
 		as->current_remaining_bytes--;
-	}
 }
 
 /* Interrupt