spi: initialize ret variable

Commit Message

Tom Rix Feb. 9, 2022, 4:30 p.m. UTC

From: Tom Rix <[e-mail]>

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 <[e-mail]>
---
 drivers/spi/spi-sunplus-sp7021.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mark Brown Feb. 9, 2022, 4:33 p.m. UTC | #1

On Wed, Feb 09, 2022 at 08:30:29AM -0800, [e-mail] wrote:
> From: Tom Rix <[e-mail]>
> 
> 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?

Tom Rix Feb. 9, 2022, 4:44 p.m. UTC | #2

On 2/9/22 8:33 AM, Mark Brown wrote:
> On Wed, Feb 09, 2022 at 08:30:29AM -0800, [e-mail] wrote:
>> From: Tom Rix <[e-mail]>
>>
>> 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

Mark Brown Feb. 9, 2022, 5:04 p.m. UTC | #3

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, [e-mail] 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.

Li-hao Kuo Feb. 10, 2022, 2:45 a.m. UTC | #4

>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.

Patch

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) {