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@xxxxxxxxx> 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@xxxxxxxxx>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202606140347.gGVWDnbi-lkp@xxxxxxxxx/
> >
> > Signed-off-by: abdelnasser hussein <abdelnasserhussein11@xxxxxxxxx>
> > ---
>
> 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
>
>