Hi Shuah,
On 11/06/2021 03:58, Shuah Khan wrote:
> Smatch static checker warns that "mdev" can be null:
>
> sound/usb/media.c:287 snd_media_device_create()
> warn: 'mdev' can also be NULL
>
> If CONFIG_MEDIA_CONTROLLER is disabled, this file should not be included
> in the build.
>
> The below conditions in the sound/usb/Makefile are in place to ensure that
> media.c isn't included in the build.
>
> sound/usb/Makefile:
> snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.o
>
> select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER &&
> (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)
>
> The following config check in include/media/media-dev-allocator.h is
> in place to enable the API only when CONFIG_MEDIA_CONTROLLER and
> CONFIG_USB are enabled.
>
> #if defined(CONFIG_MEDIA_CONTROLLER) && defined(CONFIG_USB)
>
> This check doesn't work as intended when CONFIG_USB=m. When CONFIG_USB=m,
> CONFIG_USB_MODULE is defined and CONFIG_USB is not. The above config check
> doesn't catch that CONFIG_USB is defined as a module and disables the API.
> This results in sound/usb enabling Media Controller specific ALSA driver
> code, while Media disables the Media Controller API.
>
> Fix the problem requires two changes:
>
> 1. Change the check to use IS_ENABLED to detect when CONFIG_USB is enabled
> as a module or static. Since CONFIG_MEDIA_CONTROLLER is a bool, leave
> the check unchanged to be consistent with drivers/media/Makefile.
>
> 2. Change the drivers/media/mc/Makefile to include mc-dev-allocator.o
> in mc-objs when CONFIG_USB is y or m.
If I test this patch, then I get:
drivers/media/mc/mc-dev-allocator.c:97:22: error: redefinition of 'media_device_usb_allocate'
97 | struct media_device *media_device_usb_allocate(struct usb_device *udev,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/media/mc/mc-dev-allocator.c:24:
include/media/media-dev-allocator.h:55:36: note: previous definition of 'media_device_usb_allocate' was here
55 | static inline struct media_device *media_device_usb_allocate(
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/mc/mc-dev-allocator.c:119:6: error: redefinition of 'media_device_delete'
119 | void media_device_delete(struct media_device *mdev, const char *module_name,
| ^~~~~~~~~~~~~~~~~~~
In file included from drivers/media/mc/mc-dev-allocator.c:24:
include/media/media-dev-allocator.h:59:20: note: previous definition of 'media_device_delete' was here
59 | static inline void media_device_delete(
| ^~~~~~~~~~~~~~~~~~~
The .config has:
# CONFIG_USB_SUPPORT is not set
CONFIG_MEDIA_CONTROLLER=y
Regards,
Hans
>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Link: https://lore.kernel.org/alsa-devel/YLeAvT+R22FQ%2FEyw@mwanda/
> Signed-off-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
> ---
> drivers/media/mc/Makefile | 2 +-
> include/media/media-dev-allocator.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/mc/Makefile b/drivers/media/mc/Makefile
> index 119037f0e686..140f0a78540e 100644
> --- a/drivers/media/mc/Makefile
> +++ b/drivers/media/mc/Makefile
> @@ -3,7 +3,7 @@
> mc-objs := mc-device.o mc-devnode.o mc-entity.o \
> mc-request.o
>
> -ifeq ($(CONFIG_USB),y)
> +ifeq ($(CONFIG_USB),$(filter $(CONFIG_USB),y m))
> mc-objs += mc-dev-allocator.o
> endif
>
> diff --git a/include/media/media-dev-allocator.h b/include/media/media-dev-allocator.h
> index b35ea6062596..2ab54d426c64 100644
> --- a/include/media/media-dev-allocator.h
> +++ b/include/media/media-dev-allocator.h
> @@ -19,7 +19,7 @@
>
> struct usb_device;
>
> -#if defined(CONFIG_MEDIA_CONTROLLER) && defined(CONFIG_USB)
> +#if defined(CONFIG_MEDIA_CONTROLLER) && IS_ENABLED(CONFIG_USB)
> /**
> * media_device_usb_allocate() - Allocate and return struct &media device
> *
>
