| Message ID | 20250210-soc_sdw_rt_amp-v1-1-1ee1afcd8941@ethancedwards.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: Intel: sof_sdw: initialize ret in asoc_sdw_rt_amp_spk_rtd_init() | expand |
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?
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);
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(-) --- base-commit: febbc555cf0fff895546ddb8ba2c9a523692fb55 change-id: 20250210-soc_sdw_rt_amp-e9c703ebe4dc Best regards,