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@xxxxxxxxx>
> > ---
> >  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);
>