[PATCH] ASoC: sma1307: fix uninitialized variable refence

[PATCH] ASoC: sma1307: fix uninitialized variable refence

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

When firmware loading is disabled, gcc warns that the local
'fw' variable fails to get initialized:

sound/soc/codecs/sma1307.c: In function 'sma1307_setting_loaded.isra':
sound/soc/codecs/sma1307.c:1717:12: error: 'fw' is used uninitialized [-Werror=uninitialized]
 1717 |         if (!fw) {
      |            ^
sound/soc/codecs/sma1307.c:1712:32: note: 'fw' was declared here
 1712 |         const struct firmware *fw;

Initialize this to NULL as that is what it gets checked for in
case of error.

Fixes: 576c57e6b4c1 ("ASoC: sma1307: Add driver for Iron Device SMA1307")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/soc/codecs/sma1307.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/sma1307.c b/sound/soc/codecs/sma1307.c
index 81638768ac12..9d6a70b9b626 100644
--- a/sound/soc/codecs/sma1307.c
+++ b/sound/soc/codecs/sma1307.c
@@ -1709,7 +1709,7 @@ static void sma1307_check_fault_worker(struct work_struct *work)
 
 static void sma1307_setting_loaded(struct sma1307_priv *sma1307, const char *file)
 {
-	const struct firmware *fw;
+	const struct firmware *fw = NULL;
 	int *data, size, offset, num_mode;
 
 	request_firmware(&fw, file, sma1307->dev);
-- 
2.39.5

Re: [PATCH] ASoC: sma1307: fix uninitialized variable refence

[Mark Brown]

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

On Wed, Nov 13, 2024 at 12:56:50PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When firmware loading is disabled, gcc warns that the local
> 'fw' variable fails to get initialized:
> 
> sound/soc/codecs/sma1307.c: In function 'sma1307_setting_loaded.isra':
> sound/soc/codecs/sma1307.c:1717:12: error: 'fw' is used uninitialized [-Werror=uninitialized]
>  1717 |         if (!fw) {
>       |            ^
> sound/soc/codecs/sma1307.c:1712:32: note: 'fw' was declared here
>  1712 |         const struct firmware *fw;
> 
> Initialize this to NULL as that is what it gets checked for in
> case of error.

This is just shutting the warning up - looking at the stub the real
issue is that there's a return value from request_firmware() which isn't
being checked, we're checking for the initialisation of fw instead.
There is one path in the actual implementation that returns an error
code without setting fw, though most do.  Either this caller should be
updated to check the return code or if checking fw alone is valid (which
TBH does look like the intent) the stub should be updated to set it.

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

Re: [PATCH] ASoC: sma1307: fix uninitialized variable refence

[Arnd Bergmann]

On Wed, Nov 13, 2024, at 14:24, Mark Brown wrote:
> On Wed, Nov 13, 2024 at 12:56:50PM +0100, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> When firmware loading is disabled, gcc warns that the local
>> 'fw' variable fails to get initialized:
>> 
>> sound/soc/codecs/sma1307.c: In function 'sma1307_setting_loaded.isra':
>> sound/soc/codecs/sma1307.c:1717:12: error: 'fw' is used uninitialized [-Werror=uninitialized]
>>  1717 |         if (!fw) {
>>       |            ^
>> sound/soc/codecs/sma1307.c:1712:32: note: 'fw' was declared here
>>  1712 |         const struct firmware *fw;
>> 
>> Initialize this to NULL as that is what it gets checked for in
>> case of error.
>
> This is just shutting the warning up - looking at the stub the real
> issue is that there's a return value from request_firmware() which isn't
> being checked, we're checking for the initialisation of fw instead.

I sent the updated version earlier, now just checking the return
code.

> There is one path in the actual implementation that returns an error
> code without setting fw, though most do.  Either this caller should be
> updated to check the return code or if checking fw alone is valid (which
> TBH does look like the intent) the stub should be updated to set it.

From what I saw earlier, the fw pointer gets set exactly in the
same cases that return success (through *firmware_p), checking one
or the other is almost the same, but you are totally right checking
the return code is the right thing to do here, plus it avoids
the pointless release_firmware(NULL).

       Arnd

Re: [PATCH] ASoC: sma1307: fix uninitialized variable refence

[Mark Brown]

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

On Wed, Nov 13, 2024 at 11:43:12PM +0100, Arnd Bergmann wrote:
> On Wed, Nov 13, 2024, at 14:24, Mark Brown wrote:

> > There is one path in the actual implementation that returns an error
> > code without setting fw, though most do.  Either this caller should be
> > updated to check the return code or if checking fw alone is valid (which
> > TBH does look like the intent) the stub should be updated to set it.

> From what I saw earlier, the fw pointer gets set exactly in the
> same cases that return success (through *firmware_p), checking one
> or the other is almost the same, but you are totally right checking
> the return code is the right thing to do here, plus it avoids
> the pointless release_firmware(NULL).

Yeah, the case I noticed that doesn't set firmware_p is the check for
firmware_p where it's kind of unavoidable.

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