[PATCH] spi: initialize ret variable

[PATCH] spi: initialize ret variable

[trix]

From: Tom Rix <trix@redhat.com>

Clang build fails with
spi-sunplus-sp7021.c:405:2: error: variable 'ret' is used
  uninitialized whenever switch default is taken
        default:
        ^~~~~~~

Restore initializing ret.

Fixes: 47e8fe57a66f ("spi: Modify irq request position and modify parameters")
Signed-off-by: Tom Rix <trix@redhat.com>
---
 drivers/spi/spi-sunplus-sp7021.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sunplus-sp7021.c b/drivers/spi/spi-sunplus-sp7021.c
index ba5ed9f7277a3..fc61578ce5c47 100644
--- a/drivers/spi/spi-sunplus-sp7021.c
+++ b/drivers/spi/spi-sunplus-sp7021.c
@@ -375,7 +375,7 @@ static int sp7021_spi_slave_transfer_one(struct spi_controller *ctlr, struct spi
 {
 	struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
 	struct device *dev = pspim->dev;
-	int mode, ret;
+	int mode, ret = 0;
 
 	mode = SP7021_SPI_IDLE;
 	if (xfer->tx_buf && xfer->rx_buf) {
-- 
2.26.3

Re: [PATCH] spi: initialize ret variable

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

On Wed, Feb 09, 2022 at 08:30:29AM -0800, trix@redhat.com wrote:
> From: Tom Rix <trix@redhat.com>
> 
> Clang build fails with
> spi-sunplus-sp7021.c:405:2: error: variable 'ret' is used
>   uninitialized whenever switch default is taken
>         default:
>         ^~~~~~~
> 
> Restore initializing ret.

Are you sure this is a good fix and that the compiler hasn't instead
identifed that we should be returning an error in the default case?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] spi: initialize ret variable

[Tom Rix]

On 2/9/22 8:33 AM, Mark Brown wrote:
> On Wed, Feb 09, 2022 at 08:30:29AM -0800, trix@redhat.com wrote:
>> From: Tom Rix <trix@redhat.com>
>>
>> Clang build fails with
>> spi-sunplus-sp7021.c:405:2: error: variable 'ret' is used
>>    uninitialized whenever switch default is taken
>>          default:
>>          ^~~~~~~
>>
>> Restore initializing ret.
> Are you sure this is a good fix and that the compiler hasn't instead
> identifed that we should be returning an error in the default case?

If setting mode to idle is not really a valid state, then default should 
return an error.

The old behavior returned 0 and I am treating this as a regression.

Tom

Re: [PATCH] spi: initialize ret variable

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 946 bytes --]

On Wed, Feb 09, 2022 at 08:44:21AM -0800, Tom Rix wrote:
> On 2/9/22 8:33 AM, Mark Brown wrote:
> > On Wed, Feb 09, 2022 at 08:30:29AM -0800, trix@redhat.com wrote:

> > > Clang build fails with
> > > spi-sunplus-sp7021.c:405:2: error: variable 'ret' is used
> > >    uninitialized whenever switch default is taken
> > >          default:
> > >          ^~~~~~~

> > > Restore initializing ret.

> > Are you sure this is a good fix and that the compiler hasn't instead
> > identifed that we should be returning an error in the default case?

> If setting mode to idle is not really a valid state, then default should
> return an error.

Yes, that's what I'm asking.

> The old behavior returned 0 and I am treating this as a regression.

The goal here isn't just to shut up warnings, it's to fix any problems
that they identify.  Unconditionally initialising return values is a
common way of fixing warnings while leaving real problems in place.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] spi: initialize ret variable

[郭力豪]

>Yes, that's what I'm asking.



>The original setting was ret=0; I just wanted to remove it when defining

>the ret, but it looks like it's required.

>Yes, it should return error at default



>I will submit new patch for this.




> The old behavior returned 0 and I am treating this as a regression.

The goal here isn't just to shut up warnings, it's to fix any problems
that they identify.  Unconditionally initialising return values is a
common way of fixing warnings while leaving real problems in place.


The original setting was ret=0; I just wanted to remove it when defining

the ret, but it looks like it's required.

Yes, it should return error at default



I will submit new patch for this.