[bug report] media: sound/usb: Use Media Controller API to share media resources

[Shuah Khan]

On 6/8/21 1:03 PM, Dan Carpenter wrote:
> On Tue, Jun 08, 2021 at 11:53:53AM -0600, Shuah Khan wrote:
>> On 6/2/21 6:59 AM, Dan Carpenter wrote:
>>> Hello Shuah Khan,
>>>
>>> The patch 66354f18fe5f: "media: sound/usb: Use Media Controller API
>>> to share media resources" from Apr 1, 2019, leads to the following
>>> static checker warning:
>>>
>>> 	sound/usb/media.c:287 snd_media_device_create()
>>> 	warn: 'mdev' can also be NULL
>>>
>>> sound/usb/media.c
>>>      270
>>>      271          mdev = media_device_usb_allocate(usbdev, KBUILD_MODNAME, THIS_MODULE);
>>>                   ^^^^
>>>
>>> If CONFIG_MEDIA_CONTROLLER is disabled then "mdev" is NULL.
>>
>> If CONFIG_MEDIA_CONTROLLER is disabled, this file won't be compiled.
>> Please see below clause in the Makefile.
>>
>> sound/usb/Makefile:
>> snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.o
>>
>> Also, this select in sound/usb/Kconfig selects appropriate
>> configs.
>>
>> select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER &&
>> (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)
>>
>> We are good here with the above in place to make sure media.c
>> code isn't in play when CONFIG_MEDIA_CONTROLLER is disabled.
> 
> Smatch is working based on a real .config but I misunderstood the
> problem.
> 
> include/media/media-dev-allocator.h
>      21
>      22  #if defined(CONFIG_MEDIA_CONTROLLER) && defined(CONFIG_USB)
>                                                  ^^^^^^^^^^^^^^^^^^^
> When I have CONFIG_USB=m then this is false, but when I use CONFIG_USB=y
> then this is true.  I'm not an expert on the kernel build system so I
> can't explain why defined() is not working as expected but I know that
> if we used IS_ENABLED(CONFIG_USB) that would work.
> 
> #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER) && IS_ENABLED)CONFIG_USB)
> 

Good find. IS_ENABLED is the right usage. When CONFIG_USB=m,
CONFIG_USB_MODULE is defined and CONFIG_USB isn't.

IS_ENABLED does the following and catches both, where as defined()
checks just for CONFIG_USB.

include/linux/kconfig.h     71
#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))

I will send patch to fix this. Thanks for finding it.

thanks,
-- Shuah