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@xxxxxxxxxxxxxx > > 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@xxxxxxx] > Sent: Thursday, October 05, 2017 3:17 PM > To: Wischer, Timo (ADITG/ESB) > Cc: alsa-devel@xxxxxxxxxxxxxxxx > Subject: Re: 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@xxxxxxxxxxxxxx> > 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@xxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
