* SEGFault when optininal snd_ctl_ext_callback::read_event() not set
@ 2017-10-05 12:03 Wischer, Timo (ADITG/ESB)
2017-10-05 13:16 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Wischer, Timo (ADITG/ESB) @ 2017-10-05 12:03 UTC (permalink / raw)
To: alsa-devel
Hi all,
snd_ctl_ext_callback::read_event() callback is mentioned as optional
(see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=include/control_external.h;h=12958e70a5230de9c74029c641a395a2073c8646;hb=refs/heads/master#l239)
but there is no NULL check and the NULL pointer will be called if the read_event function callback pointer is not set.
(see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/control/control_ext.c;h=56552fa1aa0ef0e6383abf4029b63944a841c2c4;hb=refs/heads/master#l419)
I think a default function has to be provided which will be called when the callback is not set or the read_event() callback should not be marked as optional.
What is your opinion?
Best regards
Timo Wischer
Advanced Driver
Information Technology GmbH
Engineering Software Base (ADITG/ESB)
Robert-Bosch-Str. 200
31139 Hildesheim
Germany
Tel. +49 5121 49 6938
Fax +49 5121 49 6999
twischer@de.adit-jv.com
ADIT is a joint venture company of Robert Bosch GmbH/Robert Bosch Car Multimedia GmbH and DENSO Corporation
Sitz: Hildesheim, Registergericht: Amtsgericht Hildesheim HRB 3438
Geschäftsführung: Wilhelm Grabow, Ken Yaguchi
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: SEGFault when optininal snd_ctl_ext_callback::read_event() not set
2017-10-05 12:03 SEGFault when optininal snd_ctl_ext_callback::read_event() not set Wischer, Timo (ADITG/ESB)
@ 2017-10-05 13:16 ` Takashi Iwai
2017-10-05 14:28 ` Wischer, Timo (ADITG/ESB)
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2017-10-05 13:16 UTC (permalink / raw)
To: Wischer, Timo (ADITG/ESB); +Cc: alsa-devel
On Thu, 05 Oct 2017 14:03:14 +0200,
Wischer, Timo (ADITG/ESB) wrote:
>
> Hi all,
>
> snd_ctl_ext_callback::read_event() callback is mentioned as optional
> (see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=include/control_external.h;h=12958e70a5230de9c74029c641a395a2073c8646;hb=refs/heads/master#l239)
>
> but there is no NULL check and the NULL pointer will be called if the read_event function callback pointer is not set.
> (see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/control/control_ext.c;h=56552fa1aa0ef0e6383abf4029b63944a841c2c4;hb=refs/heads/master#l419)
>
> I think a default function has to be provided which will be called when the callback is not set or the read_event() callback should not be marked as optional.
>
> What is your opinion?
It should have a NULL check there, as documentation clearly states
that it's optional.
Care to submit a fix patch?
thanks,
Takashi
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: SEGFault when optininal snd_ctl_ext_callback::read_event() not set
2017-10-05 13:16 ` Takashi Iwai
@ 2017-10-05 14:28 ` Wischer, Timo (ADITG/ESB)
2017-10-05 14:44 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Wischer, Timo (ADITG/ESB) @ 2017-10-05 14:28 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]
Hi Takashi,
see attached the patch.
>From my opinion failing with an error is the best solution (see explanation in the commit message)
Best regards
Timo Wischer
Advanced Driver Information Technology GmbH
Engineering Software Base (ADITG/ESB)
Robert-Bosch-Str. 200
31139 Hildesheim
Germany
Tel. +49 5121 49 6938
Fax +49 5121 49 6999
twischer@de.adit-jv.com
ADIT is a joint venture company of Robert Bosch GmbH/Robert Bosch Car Multimedia GmbH and DENSO Corporation
Sitz: Hildesheim, Registergericht: Amtsgericht Hildesheim HRB 3438
Geschäftsführung: Wilhelm Grabow, Ken Yaguchi
________________________________________
From: Takashi Iwai [tiwai@suse.de]
Sent: Thursday, October 05, 2017 3:17 PM
To: Wischer, Timo (ADITG/ESB)
Cc: alsa-devel@alsa-project.org
Subject: Re: [alsa-devel] SEGFault when optininal snd_ctl_ext_callback::read_event() not set
On Thu, 05 Oct 2017 14:03:14 +0200,
Wischer, Timo (ADITG/ESB) wrote:
>
> Hi all,
>
> snd_ctl_ext_callback::read_event() callback is mentioned as optional
> (see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=include/control_external.h;h=12958e70a5230de9c74029c641a395a2073c8646;hb=refs/heads/master#l239)
>
> but there is no NULL check and the NULL pointer will be called if the read_event function callback pointer is not set.
> (see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/control/control_ext.c;h=56552fa1aa0ef0e6383abf4029b63944a841c2c4;hb=refs/heads/master#l419)
>
> I think a default function has to be provided which will be called when the callback is not set or the read_event() callback should not be marked as optional.
>
> What is your opinion?
It should have a NULL check there, as documentation clearly states
that it's optional.
Care to submit a fix patch?
thanks,
Takashi
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: alsa-lib-ctl-ext-read-event.patch --]
[-- Type: text/x-patch; name="alsa-lib-ctl-ext-read-event.patch", Size: 1519 bytes --]
From 8cc3dbc6fad0a4de82ab4b5c95a86499c99d56d8 Mon Sep 17 00:00:00 2001
From: Timo Wischer <twischer@de.adit-jv.com>
Date: Thu, 5 Oct 2017 16:25:23 +0200
Subject: ctl: ext: Fail with error code if snd_ctl_ext_callback::read_event()
callback is not defined
The snd_ctl_ext_callback::read_event() callback is only optional
if no poll descriptor was given via
snd_ctl_ext_t::poll_fd
or
snd_ctl_ext_callback::snd_ctl_ext_poll_descriptors().
If a poll descriptor is given the
snd_ctl_ext_callback::read_event()
callback has also to be defined
because there is no minigful default behavior.
This callback will be called when ever the poll() on
the file descriptor indicates that there is an event pending.
Therefore returning a 0 which indicates that there is no event makes no
sense.
Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
diff --git a/src/control/control_ext.c b/src/control/control_ext.c
index 56552fa..d7de8e8 100644
--- a/src/control/control_ext.c
+++ b/src/control/control_ext.c
@@ -415,8 +415,12 @@ static int snd_ctl_ext_read(snd_ctl_t *handle, snd_ctl_event_t *event)
{
snd_ctl_ext_t *ext = handle->private_data;
- memset(event, 0, sizeof(*event));
- return ext->callback->read_event(ext, &event->data.elem.id, &event->data.elem.mask);
+ if (ext->callback->read_event) {
+ memset(event, 0, sizeof(*event));
+ return ext->callback->read_event(ext, &event->data.elem.id, &event->data.elem.mask);
+ }
+
+ return -EINVAL;
}
static int snd_ctl_ext_poll_descriptors_count(snd_ctl_t *handle)
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: SEGFault when optininal snd_ctl_ext_callback::read_event() not set
2017-10-05 14:28 ` Wischer, Timo (ADITG/ESB)
@ 2017-10-05 14:44 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2017-10-05 14:44 UTC (permalink / raw)
To: Wischer, Timo (ADITG/ESB); +Cc: alsa-devel
On Thu, 05 Oct 2017 16:28:33 +0200,
Wischer, Timo (ADITG/ESB) wrote:
>
> Hi Takashi,
>
> see attached the patch.
> >From my opinion failing with an error is the best solution (see explanation in the commit message)
Fair enough, applied now.
Thanks.
Takashi
>
>
> Best regards
>
> Timo Wischer
>
> Advanced Driver Information Technology GmbH
> Engineering Software Base (ADITG/ESB)
> Robert-Bosch-Str. 200
> 31139 Hildesheim
> Germany
>
> Tel. +49 5121 49 6938
> Fax +49 5121 49 6999
> twischer@de.adit-jv.com
>
> ADIT is a joint venture company of Robert Bosch GmbH/Robert Bosch Car Multimedia GmbH and DENSO Corporation
> Sitz: Hildesheim, Registergericht: Amtsgericht Hildesheim HRB 3438
> Geschäftsführung: Wilhelm Grabow, Ken Yaguchi
>
> ________________________________________
> From: Takashi Iwai [tiwai@suse.de]
> Sent: Thursday, October 05, 2017 3:17 PM
> To: Wischer, Timo (ADITG/ESB)
> Cc: alsa-devel@alsa-project.org
> Subject: Re: [alsa-devel] SEGFault when optininal snd_ctl_ext_callback::read_event() not set
>
> On Thu, 05 Oct 2017 14:03:14 +0200,
> Wischer, Timo (ADITG/ESB) wrote:
> >
> > Hi all,
> >
> > snd_ctl_ext_callback::read_event() callback is mentioned as optional
> > (see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=include/control_external.h;h=12958e70a5230de9c74029c641a395a2073c8646;hb=refs/heads/master#l239)
> >
> > but there is no NULL check and the NULL pointer will be called if the read_event function callback pointer is not set.
> > (see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/control/control_ext.c;h=56552fa1aa0ef0e6383abf4029b63944a841c2c4;hb=refs/heads/master#l419)
> >
> > I think a default function has to be provided which will be called when the callback is not set or the read_event() callback should not be marked as optional.
> >
> > What is your opinion?
>
> It should have a NULL check there, as documentation clearly states
> that it's optional.
>
> Care to submit a fix patch?
>
>
> thanks,
>
> Takashi
> From 8cc3dbc6fad0a4de82ab4b5c95a86499c99d56d8 Mon Sep 17 00:00:00 2001
> From: Timo Wischer <twischer@de.adit-jv.com>
> Date: Thu, 5 Oct 2017 16:25:23 +0200
> Subject: ctl: ext: Fail with error code if snd_ctl_ext_callback::read_event()
> callback is not defined
>
> The snd_ctl_ext_callback::read_event() callback is only optional
> if no poll descriptor was given via
> snd_ctl_ext_t::poll_fd
> or
> snd_ctl_ext_callback::snd_ctl_ext_poll_descriptors().
>
> If a poll descriptor is given the
> snd_ctl_ext_callback::read_event()
> callback has also to be defined
> because there is no minigful default behavior.
>
> This callback will be called when ever the poll() on
> the file descriptor indicates that there is an event pending.
> Therefore returning a 0 which indicates that there is no event makes no
> sense.
>
> Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
>
> diff --git a/src/control/control_ext.c b/src/control/control_ext.c
> index 56552fa..d7de8e8 100644
> --- a/src/control/control_ext.c
> +++ b/src/control/control_ext.c
> @@ -415,8 +415,12 @@ static int snd_ctl_ext_read(snd_ctl_t *handle, snd_ctl_event_t *event)
> {
> snd_ctl_ext_t *ext = handle->private_data;
>
> - memset(event, 0, sizeof(*event));
> - return ext->callback->read_event(ext, &event->data.elem.id, &event->data.elem.mask);
> + if (ext->callback->read_event) {
> + memset(event, 0, sizeof(*event));
> + return ext->callback->read_event(ext, &event->data.elem.id, &event->data.elem.mask);
> + }
> +
> + return -EINVAL;
> }
>
> static int snd_ctl_ext_poll_descriptors_count(snd_ctl_t *handle)
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-05 14:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05 12:03 SEGFault when optininal snd_ctl_ext_callback::read_event() not set Wischer, Timo (ADITG/ESB)
2017-10-05 13:16 ` Takashi Iwai
2017-10-05 14:28 ` Wischer, Timo (ADITG/ESB)
2017-10-05 14:44 ` Takashi Iwai
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.