Re: [PATCH v2] staging: greybus: audio: check sscanf() result directly
From: Dan Carpenter
Date: Mon Jun 15 2026 - 03:23:43 EST
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