[RFC PATCH] ASoC: intel/sdw_utils: Assign initial value in asoc_sdw_rt_amp_spk_rtd_init()

[RFC PATCH] ASoC: intel/sdw_utils: Assign initial value in asoc_sdw_rt_amp_spk_rtd_init()

[I Hsin Cheng]

If strstr() for codec_dai->component->name_prefix doesn't find "-1" nor
"-2", the value of ret will remain uninitialized. Initialized it with
"-EINVAL" representing the component name prefix inside "rtd" is
invalid.

If "->name_prefix" is guaranteed to have either "-1" or "-2", we can
remove the second strstr() because we know if "-1" is not in
"->name_prefix", then "-2" is in there. It'll be a waste to do one more
strstr() in that case.

Link: https://scan5.scan.coverity.com/#/project-view/36179/10063?selectedIssue=1627120
Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
---
 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 0538c252ba69..83c2368170cb 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 = -EINVAL;
 	int i;
 
 	rt_amp_map = get_codec_name_and_route(dai, codec_name);
-- 
2.43.0

Re: [RFC PATCH] ASoC: intel/sdw_utils: Assign initial value in asoc_sdw_rt_amp_spk_rtd_init()

[Liao, Bard]

On 5/5/2025 3:41 PM, I Hsin Cheng wrote:
> If strstr() for codec_dai->component->name_prefix doesn't find "-1" nor
> "-2", the value of ret will remain uninitialized. Initialized it with
> "-EINVAL" representing the component name prefix inside "rtd" is
> invalid.

Indeed. Thanks for pointing it out.

> 
> If "->name_prefix" is guaranteed to have either "-1" or "-2", we can
> remove the second strstr() because we know if "-1" is not in
> "->name_prefix", then "-2" is in there. It'll be a waste to do one more
> strstr() in that case.

The existing name_prefix is with either "-1" or "-2". But we can't make
the assumption in the asoc_sdw_rt_amp_spk_rtd_init() helper. We might
have "-3", "-4" etc in the future.

> 
> Link: https://scan5.scan.coverity.com/#/project-view/36179/10063?selectedIssue=1627120
> Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
> ---
>  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 0538c252ba69..83c2368170cb 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 = -EINVAL;
>  	int i;
>  
>  	rt_amp_map = get_codec_name_and_route(dai, codec_name);

Re: [RFC PATCH] ASoC: intel/sdw_utils: Assign initial value in asoc_sdw_rt_amp_spk_rtd_init()

[I Hsin Cheng]

On Mon, May 05, 2025 at 04:27:04PM +0800, Liao, Bard wrote:
> 
> 
> On 5/5/2025 3:41 PM, I Hsin Cheng wrote:
> > If strstr() for codec_dai->component->name_prefix doesn't find "-1" nor
> > "-2", the value of ret will remain uninitialized. Initialized it with
> > "-EINVAL" representing the component name prefix inside "rtd" is
> > invalid.
> 
> Indeed. Thanks for pointing it out.
> 
> > 
> > If "->name_prefix" is guaranteed to have either "-1" or "-2", we can
> > remove the second strstr() because we know if "-1" is not in
> > "->name_prefix", then "-2" is in there. It'll be a waste to do one more
> > strstr() in that case.
> 
> The existing name_prefix is with either "-1" or "-2". But we can't make
> the assumption in the asoc_sdw_rt_amp_spk_rtd_init() helper. We might
> have "-3", "-4" etc in the future.
>

Hi,

Thanks for your kindly review!

> The existing name_prefix is with either "-1" or "-2". But we can't make
> the assumption in the asoc_sdw_rt_amp_spk_rtd_init() helper. We might
> have "-3", "-4" etc in the future.
>
I get it, so in that case we should stick with current implementation
then. For "ret"'s initial value, do you think "-EINVAL" is ok or do you
prefer other value to be used ?
I'll refine commit message and send a formal v2 patch with your
preference.

Best regards,
I Hsin Cheng

> > 
> > Link: https://scan5.scan.coverity.com/#/project-view/36179/10063?selectedIssue=1627120
> > Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
> > ---
> >  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 0538c252ba69..83c2368170cb 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 = -EINVAL;
> >  	int i;
> >  
> >  	rt_amp_map = get_codec_name_and_route(dai, codec_name);
> 

Re: [RFC PATCH] ASoC: intel/sdw_utils: Assign initial value in asoc_sdw_rt_amp_spk_rtd_init()

[Liao, Bard]

On 5/5/2025 8:46 PM, I Hsin Cheng wrote:
> On Mon, May 05, 2025 at 04:27:04PM +0800, Liao, Bard wrote:

> 
> Hi,
> 
> Thanks for your kindly review!
> 
>> The existing name_prefix is with either "-1" or "-2". But we can't make
>> the assumption in the asoc_sdw_rt_amp_spk_rtd_init() helper. We might
>> have "-3", "-4" etc in the future.
>>
> I get it, so in that case we should stick with current implementation
> then. For "ret"'s initial value, do you think "-EINVAL" is ok or do you
> prefer other value to be used ?

-EINVAL is good to me.

> I'll refine commit message and send a formal v2 patch with your
> preference.
> 
> Best regards,
> I Hsin Cheng
>