* [bug report] media: sound/usb: Use Media Controller API to share media resources
@ 2021-06-02 12:59 Dan Carpenter
2021-06-08 17:53 ` Shuah Khan
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2021-06-02 12:59 UTC (permalink / raw)
To: shuah; +Cc: alsa-devel
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.
272 if (IS_ERR(mdev))
273 return -ENOMEM;
274
275 /* save media device - avoid lookups */
276 chip->media_dev = mdev;
277
278 snd_mixer_init:
279 /* Create media entities for mixer and control dev */
280 ret = snd_media_mixer_init(chip);
281 /* media_device might be registered, print error and continue */
282 if (ret)
283 dev_err(&usbdev->dev,
284 "Couldn't create media mixer entities. Error: %d\n",
285 ret);
286
287 if (!media_devnode_is_registered(mdev->devnode)) {
^^^^^^^^^^^^^
dereferenced without checking here
288 /* dont'register if snd_media_mixer_init() failed */
289 if (ret)
290 goto create_fail;
291
292 /* register media_device */
293 ret = media_device_register(mdev);
294 create_fail:
295 if (ret) {
296 snd_media_mixer_delete(chip);
297 media_device_delete(mdev, KBUILD_MODNAME, THIS_MODULE);
298 /* clear saved media_dev */
299 chip->media_dev = NULL;
300 dev_err(&usbdev->dev,
301 "Couldn't register media device. Error: %d\n",
302 ret);
303 return ret;
304 }
305 }
306
307 return ret;
308 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] media: sound/usb: Use Media Controller API to share media resources
2021-06-02 12:59 [bug report] media: sound/usb: Use Media Controller API to share media resources Dan Carpenter
@ 2021-06-08 17:53 ` Shuah Khan
2021-06-08 19:03 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Shuah Khan @ 2021-06-08 17:53 UTC (permalink / raw)
To: Dan Carpenter, shuah; +Cc: alsa-devel, Shuah Khan
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.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] media: sound/usb: Use Media Controller API to share media resources
2021-06-08 17:53 ` Shuah Khan
@ 2021-06-08 19:03 ` Dan Carpenter
2021-06-08 19:49 ` Shuah Khan
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2021-06-08 19:03 UTC (permalink / raw)
To: Shuah Khan; +Cc: alsa-devel, shuah
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)
23 /**
24 * media_device_usb_allocate() - Allocate and return struct &media device
25 *
26 * @udev: struct &usb_device pointer
27 * @module_name: should be filled with %KBUILD_MODNAME
28 * @owner: struct module pointer %THIS_MODULE for the driver.
29 * %THIS_MODULE is null for a built-in driver.
30 * It is safe even when %THIS_MODULE is null.
31 *
32 * This interface should be called to allocate a Media Device when multiple
33 * drivers share usb_device and the media device. This interface allocates
34 * &media_device structure and calls media_device_usb_init() to initialize
35 * it.
36 *
37 */
38 struct media_device *media_device_usb_allocate(struct usb_device *udev,
39 const char *module_name,
40 struct module *owner);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] media: sound/usb: Use Media Controller API to share media resources
2021-06-08 19:03 ` Dan Carpenter
@ 2021-06-08 19:49 ` Shuah Khan
0 siblings, 0 replies; 4+ messages in thread
From: Shuah Khan @ 2021-06-08 19:49 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel, shuah, 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-08 19:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 12:59 [bug report] media: sound/usb: Use Media Controller API to share media resources Dan Carpenter
2021-06-08 17:53 ` Shuah Khan
2021-06-08 19:03 ` Dan Carpenter
2021-06-08 19:49 ` Shuah Khan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.