[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <253ed1d8-fdbe-40ac-9791-843f3e1ca226@app.fastmail.com>
Date: Wed, 13 Nov 2024 23:43:12 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Mark Brown" <broonie@...nel.org>, "Arnd Bergmann" <arnd@...nel.org>
Cc: "Kiseok Jo" <kiseok.jo@...ndevice.com>,
"Liam Girdwood" <lgirdwood@...il.com>, "Jaroslav Kysela" <perex@...ex.cz>,
"Takashi Iwai" <tiwai@...e.com>, "Tang Bin" <tangbin@...s.chinamobile.com>,
linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ASoC: sma1307: fix uninitialized variable refence
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@...db.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
Powered by blists - more mailing lists