Linux SPI subsystem development
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
	Yang Ruibin <11162571@vivo.com>, Daniel Mack <daniel@zonque.org>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Robert Jarzmik <robert.jarzmik@free.fr>,
	linux-arm-kernel@lists.infradead.org, linux-spi@vger.kernel.org,
	linux-kernel@vger.kernel.org, opensource.kernel@vivo.com,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return
Date: Fri, 30 Aug 2024 15:23:32 -0700	[thread overview]
Message-ID: <20240830222332.GA3862110@thelio-3990X> (raw)
In-Reply-To: <0a906ee9-7b28-45e4-be67-ab3b6c5f89b1@sirena.org.uk>

On Fri, Aug 30, 2024 at 08:57:47PM +0100, Mark Brown wrote:
> On Fri, Aug 30, 2024 at 10:55:06AM +0200, Geert Uytterhoeven wrote:
> > On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@vivo.com> wrote:
> 
> > > -       if (ret)
> > > +       if (ret) {
> > > +               pci_dev_put(dma_dev);
> 
> > dma_dev is still uninitialized at this point.
> 
> I'm a bit concerned that this isn't picked up by an allmodconfig with
> the -Werror...  I'm currently using GCC 12 for that.

It shows up with -Wmaybe-uninitialized for GCC but that's disabled for
the normal kernel build with commit 78a5255ffb6a ("Stop the ad-hoc games
with -Wno-maybe-initialized"). With GCC 12:

  drivers/spi/spi-pxa2xx-pci.c: In function ‘mrfld_spi_setup’:
  drivers/spi/spi-pxa2xx-pci.c:228:17: error: ‘dma_dev’ may be used uninitialized [-Werror=maybe-uninitialized]
    228 |                 pci_dev_put(dma_dev);
        |                 ^~~~~~~~~~~~~~~~~~~~
  drivers/spi/spi-pxa2xx-pci.c:198:25: note: ‘dma_dev’ was declared here
    198 |         struct pci_dev *dma_dev;
        |                         ^~~~~~~
  drivers/spi/spi-pxa2xx-pci.c: In function ‘lpss_spi_setup’:
  drivers/spi/spi-pxa2xx-pci.c:150:17: error: ‘dma_dev’ may be used uninitialized [-Werror=maybe-uninitialized]
    150 |                 pci_dev_put(dma_dev);
        |                 ^~~~~~~~~~~~~~~~~~~~
  drivers/spi/spi-pxa2xx-pci.c:100:25: note: ‘dma_dev’ was declared here
    100 |         struct pci_dev *dma_dev;
        |                         ^~~~~~~

Clang has it under -Wuninitialized, where it was caught with a regular
allmodconfig build:

  drivers/spi/spi-pxa2xx-pci.c:150:15: error: variable 'dma_dev' is uninitialized when used here [-Werror,-Wuninitialized]
    150 |                 pci_dev_put(dma_dev);
        |                             ^~~~~~~
  drivers/spi/spi-pxa2xx-pci.c:100:25: note: initialize the variable 'dma_dev' to silence this warning
    100 |         struct pci_dev *dma_dev;
        |                                ^
        |                                 = NULL
  drivers/spi/spi-pxa2xx-pci.c:228:15: error: variable 'dma_dev' is uninitialized when used here [-Werror,-Wuninitialized]
    228 |                 pci_dev_put(dma_dev);
        |                             ^~~~~~~
  drivers/spi/spi-pxa2xx-pci.c:198:25: note: initialize the variable 'dma_dev' to silence this warning
    198 |         struct pci_dev *dma_dev;
        |                                ^
        |                                 = NULL

Perhaps a KCFLAGS=-Wmaybe-uninitialized in your make command or adding

  subdir-ccflags-$(CONFIG_CC_IS_GCC) := -Wmaybe-uninitialized

to the makefiles of the drivers that you maintain might not be a bad
idea.

Cheers,
Nathan

  reply	other threads:[~2024-08-30 22:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-29  3:35 [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return Yang Ruibin
2024-08-29 17:38 ` Mark Brown
2024-08-30  8:55 ` Geert Uytterhoeven
2024-08-30 17:10   ` Krzysztof Kozlowski
2024-08-30 17:49     ` Jakub Kicinski
2024-09-02  7:15     ` Yuesong Li
2024-09-03  6:29       ` Greg Kroah-Hartman
2024-08-30 19:12   ` Andy Shevchenko
2024-08-30 19:56     ` Mark Brown
2024-09-02 10:21       ` Andy Shevchenko
2024-08-30 19:57   ` Mark Brown
2024-08-30 22:23     ` Nathan Chancellor [this message]
2024-09-02 11:48       ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240830222332.GA3862110@thelio-3990X \
    --to=nathan@kernel.org \
    --cc=11162571@vivo.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=broonie@kernel.org \
    --cc=daniel@zonque.org \
    --cc=geert@linux-m68k.org \
    --cc=haojian.zhuang@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=opensource.kernel@vivo.com \
    --cc=robert.jarzmik@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox