Re: [PATCH] ALSA: core: Warn on empty module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/24/20 11:33 AM, Jaroslav Kysela wrote:
Dne 24. 06. 20 v 18:13 Pierre-Louis Bossart napsal(a):


On 6/24/20 11:03 AM, Takashi Iwai wrote:
The module argument passed to snd_card_new() must be a valid non-NULL
pointer when the module support is enabled.  Since ASoC driver passes
the argument from each snd_soc_card definition, one may forget to set
the owner field and lead to a NULL module easily.

For catching such an overlook, add a WARN_ON() in snd_card_new().
Also, put the card->module assignment in the ifdef block for a very
minor optimization.

Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
   sound/core/init.c | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/sound/core/init.c b/sound/core/init.c
index b02a99766351..0478847ba2b8 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -203,7 +203,10 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
       mutex_unlock(&snd_card_mutex);
       card->dev = parent;
       card->number = idx;
+#ifdef MODULE
+    WARN_ON(!module);
       card->module = module;
+#endif
       INIT_LIST_HEAD(&card->devices);
       init_rwsem(&card->controls_rwsem);
       rwlock_init(&card->ctl_files_rwlock);

Would it make sense to also change the ASoC code to use THIS_MODULE
instead of card->owner?

/* card bind complete so register a sound card */
ret = snd_card_new(card->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
        card->owner, 0, &card->snd_card);

A quick grep shows we are setting .owner = THIS_MODULE pretty much all
the time for machine drivers.


THIS_MODULE is defined when the object file is created (compile time). We want to assign the real module which creates the card here, not "snd_soc_core" which is misleading.

ok, will submit fixes for the Intel cards where this is missing.
Thanks Jaroslav for reporting this and Takashi for identifying the issue.



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux