Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753502AbcLMKzi (ORCPT ); Tue, 13 Dec 2016 05:55:38 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:51919 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750988AbcLMKzf (ORCPT ); Tue, 13 Dec 2016 05:55:35 -0500 Subject: Re: [PATCH] spi: armada-3700: fix unsigned compare than zero on irq To: Colin King , Mark Brown , linux-spi@vger.kernel.org References: <20161213102812.9029-1-colin.king@canonical.com> Cc: linux-kernel@vger.kernel.org From: Romain Perier Message-ID: Date: Tue, 13 Dec 2016 11:55:28 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20161213102812.9029-1-colin.king@canonical.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1516 Lines: 52 Hello, Le 13/12/2016 à 11:28, Colin King a écrit : > From: Colin Ian King > > 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 > --- > 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