[PATCH v2] staging: greybus: audio: check sscanf() result directly

[PATCH v2] staging: greybus: audio: check sscanf() result directly

[abdelnasser hussein]

Smatch warns:

  drivers/staging/greybus/audio_codec.c:335 gbaudio_module_update()
  warn: sscanf doesn't return error codes

sscanf() returns the number of successfully matched input items, not a
negative error code. Compare the return value directly with the expected
number of conversions instead of storing it in ret as an error code.

Also remove the redundant else-if check for snd_soc_dapm_aif_out. The
widget id is validated earlier in the function, so the remaining branch
can only handle snd_soc_dapm_aif_out. This avoids a compiler warning
about a potentially uninitialized variable.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202606140347.gGVWDnbi-lkp@intel.com/

Signed-off-by: abdelnasser hussein <abdelnasserhussein11@gmail.com>
---
 drivers/staging/greybus/audio_codec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
index 720aa752e17e..6daa4e706792 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -311,8 +311,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec,
 	}
 
 	/* parse dai_id from AIF widget's stream_name */
-	ret = sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir);
-	if (ret < 3) {
+	if (sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir) != 3) {
 		dev_err(codec->dev, "Error while parsing dai_id for %s\n", w->name);
 		return -EINVAL;
 	}
@@ -323,7 +322,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec,
 			ret = gbaudio_module_enable_tx(codec, module, dai_id);
 		else
 			ret = gbaudio_module_disable_tx(module, dai_id);
-	} else if (w->id == snd_soc_dapm_aif_out) {
+	} else {
 		if (enable)
 			ret = gbaudio_module_enable_rx(codec, module, dai_id);
 		else
-- 
2.54.0

Re: [PATCH v2] staging: greybus: audio: check sscanf() result directly

[Greg Kroah-Hartman]

On Sun, Jun 14, 2026 at 09:08:57AM +0300, abdelnasser hussein wrote:
> Smatch warns:
> 
>   drivers/staging/greybus/audio_codec.c:335 gbaudio_module_update()
>   warn: sscanf doesn't return error codes
> 
> sscanf() returns the number of successfully matched input items, not a
> negative error code. Compare the return value directly with the expected
> number of conversions instead of storing it in ret as an error code.
> 
> Also remove the redundant else-if check for snd_soc_dapm_aif_out. The
> widget id is validated earlier in the function, so the remaining branch
> can only handle snd_soc_dapm_aif_out. This avoids a compiler warning
> about a potentially uninitialized variable.

When you say "also" that implies it should be a separate patch.

> 
> Reported-by: kernel test robot <lkp@intel.com>

lkp didn't report the smatch warning :(

> Closes: https://lore.kernel.org/oe-kbuild-all/202606140347.gGVWDnbi-lkp@intel.com/
> 
> Signed-off-by: abdelnasser hussein <abdelnasserhussein11@gmail.com>
> ---
>  drivers/staging/greybus/audio_codec.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

No list of what changed from previous versions?

thanks,

greg k-h

Re: [PATCH v2] staging: greybus: audio: check sscanf() result directly

[nasser]

Hi Greg,

Thanks for the review and the feedback.

On Sun, Jun 14, 2026 at 06:15:00AM +0000, Greg Kroah-Hartman wrote:
> When you say "also" that implies it should be a separate patch.

Understood. I will split these changes into a two-patch series in v3.

> lkp didn't report the smatch warning :(

My mistake. I will fix the Reported-by and Closes tags in the next
version to correctly reflect the source of the warning.

> No list of what changed from previous versions?

I apologize for missing that. I will make sure to include a proper
changelog below the '---' line for v3.

I will send the v3 patch series shortly.

Thanks,
Abdelnasser


On Sun, Jun 14, 2026 at 9:17 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jun 14, 2026 at 09:08:57AM +0300, abdelnasser hussein wrote:
> > Smatch warns:
> >
> >   drivers/staging/greybus/audio_codec.c:335 gbaudio_module_update()
> >   warn: sscanf doesn't return error codes
> >
> > sscanf() returns the number of successfully matched input items, not a
> > negative error code. Compare the return value directly with the expected
> > number of conversions instead of storing it in ret as an error code.
> >
> > Also remove the redundant else-if check for snd_soc_dapm_aif_out. The
> > widget id is validated earlier in the function, so the remaining branch
> > can only handle snd_soc_dapm_aif_out. This avoids a compiler warning
> > about a potentially uninitialized variable.
>
> When you say "also" that implies it should be a separate patch.
>
> >
> > Reported-by: kernel test robot <lkp@intel.com>
>
> lkp didn't report the smatch warning :(
>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202606140347.gGVWDnbi-lkp@intel.com/
> >
> > Signed-off-by: abdelnasser hussein <abdelnasserhussein11@gmail.com>
> > ---
> >  drivers/staging/greybus/audio_codec.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
>
> No list of what changed from previous versions?
>
> thanks,
>
> greg k-h

Re: [PATCH v2] staging: greybus: audio: check sscanf() result directly

[Dan Carpenter]

On Sun, Jun 14, 2026 at 09:08:57AM +0300, abdelnasser hussein wrote:
> Smatch warns:
> 
>   drivers/staging/greybus/audio_codec.c:335 gbaudio_module_update()
>   warn: sscanf doesn't return error codes
> 
> sscanf() returns the number of successfully matched input items, not a
> negative error code. Compare the return value directly with the expected
> number of conversions instead of storing it in ret as an error code.
> 
> Also remove the redundant else-if check for snd_soc_dapm_aif_out. The
> widget id is validated earlier in the function, so the remaining branch
> can only handle snd_soc_dapm_aif_out. This avoids a compiler warning
> about a potentially uninitialized variable.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202606140347.gGVWDnbi-lkp@intel.com/
> 
> Signed-off-by: abdelnasser hussein <abdelnasserhussein11@gmail.com>
> ---

This static checker warning is triggered when we propagate the return
from sscanf().

>  drivers/staging/greybus/audio_codec.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> index 720aa752e17e..6daa4e706792 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -311,8 +311,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec,
>  	}
>  
>  	/* parse dai_id from AIF widget's stream_name */
> -	ret = sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir);
> -	if (ret < 3) {
> +	if (sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir) != 3) {
>  		dev_err(codec->dev, "Error while parsing dai_id for %s\n", w->name);
>  		return -EINVAL;

So this code is fine as-is since it's returning -EINVAL.

>  	}
> @@ -323,7 +322,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec,
>  			ret = gbaudio_module_enable_tx(codec, module, dai_id);
>  		else
>  			ret = gbaudio_module_disable_tx(module, dai_id);
> -	} else if (w->id == snd_soc_dapm_aif_out) {
> +	} else {

Yes, this is what the static checker is complaining about.  It thinks
that the if statement might be false.  But to a human reader, the
else if or the plain else are equivalent.  It's just a style choice
which way to write it.

I would just leave this as-is since the original code is fine.  To be
honest, most old static checker warnings are stuff that someone
already decided to leave as is.

It's better to write new checks.  Just look for simple fixes in git
log and then you can vibe code a check.  Better to work against the
devel branch of smatch.

regards,
dan carpenter

>  		if (enable)
>  			ret = gbaudio_module_enable_rx(codec, module, dai_id);
>  		else

Re: [PATCH v2] staging: greybus: audio: check sscanf() result directly

[nasser]

Hi Dan,

Thanks a lot for the explanation and the valuable information. I will
definitely take your advice, learn from this, and focus on tackling
more important issues going forward.

Best regards,
Abdelnasser


On Mon, Jun 15, 2026 at 10:21 AM Dan Carpenter <error27@gmail.com> wrote:
>
> On Sun, Jun 14, 2026 at 09:08:57AM +0300, abdelnasser hussein wrote:
> > Smatch warns:
> >
> >   drivers/staging/greybus/audio_codec.c:335 gbaudio_module_update()
> >   warn: sscanf doesn't return error codes
> >
> > sscanf() returns the number of successfully matched input items, not a
> > negative error code. Compare the return value directly with the expected
> > number of conversions instead of storing it in ret as an error code.
> >
> > Also remove the redundant else-if check for snd_soc_dapm_aif_out. The
> > widget id is validated earlier in the function, so the remaining branch
> > can only handle snd_soc_dapm_aif_out. This avoids a compiler warning
> > about a potentially uninitialized variable.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202606140347.gGVWDnbi-lkp@intel.com/
> >
> > Signed-off-by: abdelnasser hussein <abdelnasserhussein11@gmail.com>
> > ---
>
> This static checker warning is triggered when we propagate the return
> from sscanf().
>
> >  drivers/staging/greybus/audio_codec.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> > index 720aa752e17e..6daa4e706792 100644
> > --- a/drivers/staging/greybus/audio_codec.c
> > +++ b/drivers/staging/greybus/audio_codec.c
> > @@ -311,8 +311,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec,
> >       }
> >
> >       /* parse dai_id from AIF widget's stream_name */
> > -     ret = sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir);
> > -     if (ret < 3) {
> > +     if (sscanf(w->sname, "%s %d %s", intf_name, &dai_id, dir) != 3) {
> >               dev_err(codec->dev, "Error while parsing dai_id for %s\n", w->name);
> >               return -EINVAL;
>
> So this code is fine as-is since it's returning -EINVAL.
>
> >       }
> > @@ -323,7 +322,7 @@ int gbaudio_module_update(struct gbaudio_codec_info *codec,
> >                       ret = gbaudio_module_enable_tx(codec, module, dai_id);
> >               else
> >                       ret = gbaudio_module_disable_tx(module, dai_id);
> > -     } else if (w->id == snd_soc_dapm_aif_out) {
> > +     } else {
>
> Yes, this is what the static checker is complaining about.  It thinks
> that the if statement might be false.  But to a human reader, the
> else if or the plain else are equivalent.  It's just a style choice
> which way to write it.
>
> I would just leave this as-is since the original code is fine.  To be
> honest, most old static checker warnings are stuff that someone
> already decided to leave as is.
>
> It's better to write new checks.  Just look for simple fixes in git
> log and then you can vibe code a check.  Better to work against the
> devel branch of smatch.
>
> regards,
> dan carpenter
>
> >               if (enable)
> >                       ret = gbaudio_module_enable_rx(codec, module, dai_id);
> >               else
>
>