From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data Date: Mon, 09 Feb 2015 17:18:49 +0100 Message-ID: References: <1423494126-24054-1-git-send-email-o-takashi@sakamocchi.jp> <54D8D9CB.5080709@sakamocchi.jp> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 927B0260506 for ; Mon, 9 Feb 2015 17:18:52 +0100 (CET) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Sakamoto Cc: alsa-devel@alsa-project.org, clemens@ladisch.de List-Id: alsa-devel@alsa-project.org At Mon, 09 Feb 2015 17:02:51 +0100, Takashi Iwai wrote: > > At Tue, 10 Feb 2015 01:01:15 +0900, > Takashi Sakamoto wrote: > > > > On Feb 10 2015 00:28, Takashi Iwai wrote: > > > At Tue, 10 Feb 2015 00:02:06 +0900, > > > Takashi Sakamoto wrote: > > >> > > >> When event originator doesn't set numerical ID in identical information, > > >> the event data includes no numerical ID, thus userspace applications > > >> cannot identify the control just by unique ID in event data. > > >> > > >> This commit fix this bug so as the event data includes all of identical > > >> information. > > >> > > >> Signed-off-by: Takashi Sakamoto > > > > > > You can't overwrite id pointer in this case. It's a caller's object. > > > > Exactly. I missed it... > > > > > One way to fix would be to copy the id instance. Another way would be > > > to change up/down_write() with _read(), and include snd_ctl_notify() > > > call with kctl->id inside the semaphore lock. > > > > > > The former would be less changes but consume the stack significantly. > > > > If any sub-effects are allowed, we can use caller's data via the > > pointer, like: > > memcpy(id, &kctl->id, sizeof(struct snd_ctl_elem_id); > > Then: > > snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_INFO, id); > > That's what I suggested as the first example. ... and I think this is a better way in the end. Simpler, better. The stack usage is very likely acceptable. Takashi