* [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data
@ 2015-02-09 15:02 Takashi Sakamoto
2015-02-09 15:28 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Sakamoto @ 2015-02-09 15:02 UTC (permalink / raw)
To: o-takashi, tiwai, clemens; +Cc: alsa-devel
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 <o-takashi@sakamocchi.jp>
---
sound/core/control.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/core/control.c b/sound/core/control.c
index 6a72b3e..884fddd 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -587,6 +587,7 @@ int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id,
}
ret = 1;
unlock:
+ *id = kctl->id;
up_write(&card->controls_rwsem);
if (ret > 0)
snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_INFO, id);
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data
2015-02-09 15:02 [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data Takashi Sakamoto
@ 2015-02-09 15:28 ` Takashi Iwai
2015-02-09 16:01 ` Takashi Sakamoto
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2015-02-09 15:28 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens
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 <o-takashi@sakamocchi.jp>
You can't overwrite id pointer in this case. It's a caller's object.
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.
thanks,
Takashi
> ---
> sound/core/control.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/sound/core/control.c b/sound/core/control.c
> index 6a72b3e..884fddd 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -587,6 +587,7 @@ int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id,
> }
> ret = 1;
> unlock:
> + *id = kctl->id;
> up_write(&card->controls_rwsem);
> if (ret > 0)
> snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_INFO, id);
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data
2015-02-09 15:28 ` Takashi Iwai
@ 2015-02-09 16:01 ` Takashi Sakamoto
2015-02-09 16:02 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Sakamoto @ 2015-02-09 16:01 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, clemens
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 <o-takashi@sakamocchi.jp>
>
> 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);
But this is not better. Mmm...
> thanks,
>
> Takashi
>
>
>> ---
>> sound/core/control.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/sound/core/control.c b/sound/core/control.c
>> index 6a72b3e..884fddd 100644
>> --- a/sound/core/control.c
>> +++ b/sound/core/control.c
>> @@ -587,6 +587,7 @@ int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id,
>> }
>> ret = 1;
>> unlock:
>> + *id = kctl->id;
>> up_write(&card->controls_rwsem);
>> if (ret > 0)
>> snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_INFO, id);
>> --
>> 2.1.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data
2015-02-09 16:01 ` Takashi Sakamoto
@ 2015-02-09 16:02 ` Takashi Iwai
2015-02-09 16:18 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2015-02-09 16:02 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens
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 <o-takashi@sakamocchi.jp>
> >
> > 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.
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data
2015-02-09 16:02 ` Takashi Iwai
@ 2015-02-09 16:18 ` Takashi Iwai
2015-02-10 5:24 ` Takashi Sakamoto
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2015-02-09 16:18 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel, clemens
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 <o-takashi@sakamocchi.jp>
> > >
> > > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data
2015-02-09 16:18 ` Takashi Iwai
@ 2015-02-10 5:24 ` Takashi Sakamoto
0 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2015-02-10 5:24 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, clemens
On Feb 10 2015 01:18, Takashi Iwai wrote:
> 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 <o-takashi@sakamocchi.jp>
>>>>
>>>> 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.
I misunderstood to use heap.
> ... and I think this is a better way in the end. Simpler, better.
> The stack usage is very likely acceptable.
I think so. I'll submit again with some comments about the sub-effect to
callers.
Thanks
Takashi Sakamoto
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-02-10 5:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-09 15:02 [PATCH] ALSA: control: confirm to return all identical information in 'activate' event data Takashi Sakamoto
2015-02-09 15:28 ` Takashi Iwai
2015-02-09 16:01 ` Takashi Sakamoto
2015-02-09 16:02 ` Takashi Iwai
2015-02-09 16:18 ` Takashi Iwai
2015-02-10 5:24 ` Takashi Sakamoto
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox