Le 15/06/2023 à 04:17, Su Hui a écrit :
smatch error:
sound/pci/ac97/ac97_codec.c:2354 snd_ac97_mixer() error:
we previously assumed 'rac97' could be null (see line 2072)
remove redundant assignment, return error if rac97 is NULL.
Hi,
why is the assigment redundant?
Should an error occur, the 'struct snd_ac97 **' parameter was garanted
to be set to NULL, now it is left as-is.
I've checked all callers and apparently this is fine because the probes
fail if snd_ac97_mixer() returns an error.
However, some drivers with several mixers seem to rely on the value
being NULL in case of error.
See [1] as an example of such code that forces a NULL value on its own,
to be sure.
So, wouldn't it be safer to leave a "*rac97 = NULL;" just after the
added sanity check?
CJ
[1]:
https://elixir.bootlin.com/linux/v6.5-rc7/source/sound/pci/atiixp.c#L1438
Fixes: da3cec35dd3c ("ALSA: Kill snd_assert() in sound/pci/*")
Signed-off-by: Su Hui <suhui@xxxxxxxxxxxx>
---
sound/pci/ac97/ac97_codec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
index 9afc5906d662..80a65b8ad7b9 100644
--- a/sound/pci/ac97/ac97_codec.c
+++ b/sound/pci/ac97/ac97_codec.c
@@ -2069,8 +2069,8 @@ int snd_ac97_mixer(struct snd_ac97_bus *bus, struct snd_ac97_template *template,
.dev_disconnect = snd_ac97_dev_disconnect,
};
- if (rac97)
- *rac97 = NULL;
+ if (!rac97)
+ return -EINVAL;
if (snd_BUG_ON(!bus || !template))
return -EINVAL;
if (snd_BUG_ON(template->num >= 4))
