Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

Nathan Chancellor
2024-08-30T22:23:34Z

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