| Message ID | B256D81BAE5131468A838E5D7A2436417301AE8C@penmbx01 (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0c3b97487d15a452f1fa7bfd9b6f1bd1a31d538d |
| Headers | show |
2014-03-27 9:13 GMT+08:00 Yang, Wenyou <Wenyou.Yang@atmel.com>: > >> -----Original Message----- >> From: Axel Lin [mailto:axel.lin@ingics.com] >> Sent: Thursday, March 27, 2014 8:46 AM >> To: Mark Brown >> Cc: Yang, Wenyou; linux-spi >> Subject: Re: [PATCH] spi: atmel: Fix test unsigned var < 0 >> >> 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? > > Like Mark said, It seems it is more reasonable using a signed int, instead of unsigned int. OK. I'm sending a formal patch with your Signed-off-by now. 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
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c index 8005f98..485e6cc 100644 --- a/drivers/spi/spi-atmel.c +++ b/drivers/spi/spi-atmel.c @@ -224,7 +224,7 @@ struct atmel_spi { struct platform_device *pdev; struct spi_transfer *current_transfer; - unsigned long current_remaining_bytes; + int current_remaining_bytes; int done_status; struct completion xfer_completion; @@ -1110,6 +1110,8 @@ static int atmel_spi_one_transfer(struct spi_master *master, atmel_spi_next_xfer_pio(master, xfer); } else { as->current_remaining_bytes -= len; + if (as->current_remaining_bytes < 0) + as->current_remaining_bytes = 0; } } else { atmel_spi_next_xfer_pio(master, xfer);