diff mbox series

spi: initialize status to success

Message ID 20220416205132.295779-1-trix@redhat.com (mailing list archive)
State New, archived
Headers show
Series spi: initialize status to success | expand

Commit Message

Tom Rix April 16, 2022, 8:51 p.m. UTC
Clang static analysis reports this issue
spi.c:3583:2: warning: 2nd function call argument
  is an uninitialized value
  trace_spi_setup(spi, status);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

It is possible that status is never set because
its function level setting was removed by the
commit listed in the Fixes: tag.

Assume that setting 8 bits-per-word is ok and
initialize the status to 0.

Fixes: b3fe2e516741 ("spi: core: Only check bits_per_word validity when explicitly provided")
Signed-off-by: Tom Rix <trix@redhat.com>
---
 drivers/spi/spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nathan Chancellor April 16, 2022, 11:05 p.m. UTC | #1
On Sat, Apr 16, 2022 at 04:51:32PM -0400, Tom Rix wrote:
> Clang static analysis reports this issue
> spi.c:3583:2: warning: 2nd function call argument
>   is an uninitialized value
>   trace_spi_setup(spi, status);
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> It is possible that status is never set because
> its function level setting was removed by the
> commit listed in the Fixes: tag.
> 
> Assume that setting 8 bits-per-word is ok and
> initialize the status to 0.
> 
> Fixes: b3fe2e516741 ("spi: core: Only check bits_per_word validity when explicitly provided")
> Signed-off-by: Tom Rix <trix@redhat.com>

I am very surprised that -Wsometimes-uninitialized does not trigger
here, as this instance does not require interprocedural analysis, right?
status is not passed by reference as far as I can tell and it is only
initialized in if statements now but it is unconditionally used at the
end of the function. Maybe it does not matter too much, given that
auto-initialization is turned on by default when supported.

Regardless, this fix looks fine. I do not really like initializing
variables at the top of the function as it can hide error codes but it
is always a maintainer preference anyways so:

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  drivers/spi/spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 563a56ce34a0..890ff46c784a 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -3475,7 +3475,7 @@ static int __spi_validate_bits_per_word(struct spi_controller *ctlr,
>  int spi_setup(struct spi_device *spi)
>  {
>  	unsigned	bad_bits, ugly_bits;
> -	int		status;
> +	int		status = 0;
>  
>  	/*
>  	 * Check mode to prevent that any two of DUAL, QUAD and NO_MOSI/MISO
> -- 
> 2.27.0
>
Mark Brown April 17, 2022, 7:37 p.m. UTC | #2
On Sat, Apr 16, 2022 at 04:51:32PM -0400, Tom Rix wrote:
> Clang static analysis reports this issue
> spi.c:3583:2: warning: 2nd function call argument

The same patch was sent and applied several days ago?  
73f93db5c49b9f52c902e5dc6c750bf9832e0450
diff mbox series

Patch

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 563a56ce34a0..890ff46c784a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3475,7 +3475,7 @@  static int __spi_validate_bits_per_word(struct spi_controller *ctlr,
 int spi_setup(struct spi_device *spi)
 {
 	unsigned	bad_bits, ugly_bits;
-	int		status;
+	int		status = 0;
 
 	/*
 	 * Check mode to prevent that any two of DUAL, QUAD and NO_MOSI/MISO