There is a possibility for an uninitialized *ret* variable to be
returned in some code paths.

Setting to 0 prevents a random value from being returned.

Closes: https://scan5.scan.coverity.com/#/project-view/63873/10063?selectedIssue=1627120
Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
---
Would it potentially be better to remove ret entirely and just return 0
explicitly at the end of the function and directly return in the if
statements? I'm not sure.
---
 sound/soc/sdw_utils/soc_sdw_rt_amp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/sdw_utils/soc_sdw_rt_amp.c b/sound/soc/sdw_utils/soc_sdw_rt_amp.c
index 0538c252ba69b1f5a24ba2de2a610b22d0c0665f..61a00a3548d85689c13e2d2a301da17a572e4a0e 100644
--- a/sound/soc/sdw_utils/soc_sdw_rt_amp.c
+++ b/sound/soc/sdw_utils/soc_sdw_rt_amp.c
@@ -190,7 +190,7 @@ int asoc_sdw_rt_amp_spk_rtd_init(struct snd_soc_pcm_runtime *rtd, struct snd_soc
 	const struct snd_soc_dapm_route *rt_amp_map;
 	char codec_name[CODEC_NAME_SIZE];
 	struct snd_soc_dai *codec_dai;
-	int ret;
+	int ret = 0;
 	int i;
 
 	rt_amp_map = get_codec_name_and_route(dai, codec_name);

---
base-commit: febbc555cf0fff895546ddb8ba2c9a523692fb55
change-id: 20250210-soc_sdw_rt_amp-e9c703ebe4dc

Best regards,
-- 
Ethan Carter Edwards <ethan@ethancedwards.com>

On Mon, Feb 10, 2025 at 11:08:27PM -0500, Ethan Carter Edwards wrote:
> There is a possibility for an uninitialized *ret* variable to be
> returned in some code paths.
> 
> Setting to 0 prevents a random value from being returned.

That'll shut up the warning but is the warning trying to tell us that
there's a logic bug somewhere in the function and we're for example
forgetting to look at a return value in some path in the function?

On 2/11/25 07:13, Mark Brown wrote:
> On Mon, Feb 10, 2025 at 11:08:27PM -0500, Ethan Carter Edwards wrote:
>> There is a possibility for an uninitialized *ret* variable to be
>> returned in some code paths.
>>
>> Setting to 0 prevents a random value from being returned.
> 
> That'll shut up the warning but is the warning trying to tell us that
> there's a logic bug somewhere in the function and we're for example
> forgetting to look at a return value in some path in the function?

The problematic code is this:

for_each_rtd_codec_dais(rtd, i, codec_dai) {
	if (strstr(codec_dai->component->name_prefix, "-1"))
		ret = snd_soc_dapm_add_routes(&card->dapm, rt_amp_map, 2);
	else if (strstr(codec_dai->component->name_prefix, "-2"))
		ret = snd_soc_dapm_add_routes(&card->dapm, rt_amp_map + 2, 2);
}

return ret;

I am not sure if it's possible that either the for_each does nothing or that the two branches are skipped, but certainly initializing the 'ret' value makes sense to me.

Bard, Shuming, what do you think?

On Tue, Feb 11, 2025 at 12:54:52PM -0600, Pierre-Louis Bossart wrote:
> On 2/11/25 07:13, Mark Brown wrote:
> > On Mon, Feb 10, 2025 at 11:08:27PM -0500, Ethan Carter Edwards wrote:
> >> There is a possibility for an uninitialized *ret* variable to be
> >> returned in some code paths.
> >>
> >> Setting to 0 prevents a random value from being returned.
> > 
> > That'll shut up the warning but is the warning trying to tell us that
> > there's a logic bug somewhere in the function and we're for example
> > forgetting to look at a return value in some path in the function?
> 
> The problematic code is this:
> 
> for_each_rtd_codec_dais(rtd, i, codec_dai) {
> 	if (strstr(codec_dai->component->name_prefix, "-1"))
> 		ret = snd_soc_dapm_add_routes(&card->dapm, rt_amp_map, 2);
> 	else if (strstr(codec_dai->component->name_prefix, "-2"))
> 		ret = snd_soc_dapm_add_routes(&card->dapm, rt_amp_map + 2, 2);
> }
> 
> return ret;
> 
> I am not sure if it's possible that either the for_each does nothing or that the two branches are skipped, but certainly initializing the 'ret' value makes sense to me.
> 
> Bard, Shuming, what do you think?

I'm just skimming through patchwork and this patch doesn't seem to have
made any progress. What're next steps?

-Kees

-- 
Kees Cook