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

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

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@suse.de>
>>> ---
>>>    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.