Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1541002

Re: [PATCH] spi: armada-3700: fix unsigned compare than zero on irq

From Colin Ian King <colin.king@canonical.com>
Newsgroups linux.kernel
Subject Re: [PATCH] spi: armada-3700: fix unsigned compare than zero on irq
Date 2016-12-13 12:10 +0100
Message-ID <sNT85-tf-9@gated-at.bofh.it> (permalink)
References <sNSvo-8sJ-17@gated-at.bofh.it> <sNSYq-aO-47@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 13/12/16 10:55, Romain Perier wrote:
> Hello,
> 
> Le 13/12/2016 à 11:28, Colin King a écrit :
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> spi->irq is an unsigned integer hence the check if status is less than
>> zero has no effect.  Fix this by replacing spi->irq with an int irq
>> so the less than zero compare will correctly detect errors.
>>
>> Issue found with static analysis with CoverityScan, CID1388567
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  drivers/spi/spi-armada-3700.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/spi/spi-armada-3700.c
>> b/drivers/spi/spi-armada-3700.c
>> index e89da0a..4e92178 100644
>> --- a/drivers/spi/spi-armada-3700.c
>> +++ b/drivers/spi/spi-armada-3700.c
>> @@ -800,7 +800,7 @@ static int a3700_spi_probe(struct platform_device
>> *pdev)
>>      struct spi_master *master;
>>      struct a3700_spi *spi;
>>      u32 num_cs = 0;
>> -    int ret = 0;
>> +    int irq, ret = 0;
>>
>>      master = spi_alloc_master(dev, sizeof(*spi));
>>      if (!master) {
>> @@ -846,12 +846,13 @@ static int a3700_spi_probe(struct
>> platform_device *pdev)
>>          goto error;
>>      }
>>
>> -    spi->irq = platform_get_irq(pdev, 0);
>> -    if (spi->irq < 0) {
>> -        dev_err(dev, "could not get irq: %d\n", spi->irq);
>> +    irq = platform_get_irq(pdev, 0);
>> +    if (irq < 0) {
>> +        dev_err(dev, "could not get irq: %d\n", irq);
>>          ret = -ENXIO;
>>          goto error;
>>      }
>> +    spi->irq = irq;
>>
>>      init_completion(&spi->done);
>>
>>
> 
> Why don't you change only the type of "irq" in struct a3700_spi ?
> 
> Thanks,
> Romain

..because I've observed that a lot of driver seems to use a unsigned int
for the irq in their driver specific structs and I also didn't want to
modify this to an int in case it some subtle signed/unsigned logic
somewhere else that I was not aware of. Just seemed like the least risky
fix to me.

Colin

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] spi: armada-3700: fix unsigned compare than zero on irq Colin King <colin.king@canonical.com> - 2016-12-13 11:30 +0100
  Re: [PATCH] spi: armada-3700: fix unsigned compare than zero on irq Romain Perier <romain.perier@free-electrons.com> - 2016-12-13 12:00 +0100
    Re: [PATCH] spi: armada-3700: fix unsigned compare than zero on irq Romain Perier <romain.perier@free-electrons.com> - 2016-12-13 12:10 +0100
    Re: [PATCH] spi: armada-3700: fix unsigned compare than zero on irq Colin Ian King <colin.king@canonical.com> - 2016-12-13 12:10 +0100
  Applied "spi: armada-3700: fix unsigned compare than zero on irq" to the spi tree Mark Brown <broonie@kernel.org> - 2016-12-14 19:10 +0100

csiph-web