[alsa-devel] [PATCH] ALSA: control: fix failure to return numerical ID in event data
Currently an instance of control has zero as its member for numerical ID even if any IDs are assigned to. According to this bug, any userspace applications cannot identify controls by the ID when handling any events. On the other hand, the other members such as name are still valid, therefore applications can identify controls without fixing this.
This is not preferable because the ID has an advantage to allow userspace applications to distinguish each controls by itself, without any combinations.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/control.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/sound/core/control.c b/sound/core/control.c index 60caba1..d482997 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -352,6 +352,9 @@ int snd_ctl_add(struct snd_card *card, struct snd_kcontrol *kcontrol) if (id.index > UINT_MAX - kcontrol->count) goto error;
+ /* Any ID numbers are not assigned to this control yet. */ + id.numid = 0; + down_write(&card->controls_rwsem); if (snd_ctl_find_id(card, &id)) { up_write(&card->controls_rwsem); @@ -369,12 +372,17 @@ int snd_ctl_add(struct snd_card *card, struct snd_kcontrol *kcontrol) err = -ENOMEM; goto error; } - list_add_tail(&kcontrol->list, &card->controls); - card->controls_count += kcontrol->count; + + /* Now this control is allowed to have ID number. */ kcontrol->id.numid = card->last_numid + 1; card->last_numid += kcontrol->count; - count = kcontrol->count; + card->controls_count += kcontrol->count; + list_add_tail(&kcontrol->list, &card->controls); + up_write(&card->controls_rwsem); + + id = kcontrol->id; + count = kcontrol->count; for (idx = 0; idx < count; idx++, id.index++, id.numid++) snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_ADD, &id); return 0;
At Sun, 8 Feb 2015 19:28:56 +0900, Takashi Sakamoto wrote:
Currently an instance of control has zero as its member for numerical ID even if any IDs are assigned to. According to this bug, any userspace applications cannot identify controls by the ID when handling any events. On the other hand, the other members such as name are still valid, therefore applications can identify controls without fixing this.
This is not preferable because the ID has an advantage to allow userspace applications to distinguish each controls by itself, without any combinations.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
You're trying to address two individual bugs. Please split. The first bug is the missing zero clear of id.numid, and the second bug is the missing numid set for notification. Both can be oneliner. The latter would be something like:
--- a/sound/core/control.c +++ b/sound/core/control.c @@ -371,7 +371,7 @@ int snd_ctl_add(struct snd_card *card, struct snd_kcontrol *kcontrol) } list_add_tail(&kcontrol->list, &card->controls); card->controls_count += kcontrol->count; - kcontrol->id.numid = card->last_numid + 1; + id.numid = kcontrol->id.numid = card->last_numid + 1; card->last_numid += kcontrol->count; count = kcontrol->count; up_write(&card->controls_rwsem);
If you want to reorder the calls and add the comments, do it in another patch to make clear that it's just a refactoring. Don't mix up refactoring and a fix, unless it's really trivial.
thanks,
Takashi
On Feb 8 2015 20:09, Takashi Iwai wrote:
At Sun, 8 Feb 2015 19:28:56 +0900, Takashi Sakamoto wrote:
Currently an instance of control has zero as its member for numerical ID even if any IDs are assigned to. According to this bug, any userspace applications cannot identify controls by the ID when handling any events. On the other hand, the other members such as name are still valid, therefore applications can identify controls without fixing this.
This is not preferable because the ID has an advantage to allow userspace applications to distinguish each controls by itself, without any combinations.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
You're trying to address two individual bugs. Please split. The first bug is the missing zero clear of id.numid, and the second bug is the missing numid set for notification. Both can be oneliner. The latter would be something like:
--- a/sound/core/control.c +++ b/sound/core/control.c @@ -371,7 +371,7 @@ int snd_ctl_add(struct snd_card *card, struct snd_kcontrol *kcontrol) } list_add_tail(&kcontrol->list, &card->controls); card->controls_count += kcontrol->count;
- kcontrol->id.numid = card->last_numid + 1;
- id.numid = kcontrol->id.numid = card->last_numid + 1; card->last_numid += kcontrol->count; count = kcontrol->count; up_write(&card->controls_rwsem);
If you want to reorder the calls and add the comments, do it in another patch to make clear that it's just a refactoring. Don't mix up refactoring and a fix, unless it's really trivial.
OK. In next patch, I'll remove the refactoring codes (including zero-clear) because it depends on developers' style and easy to bring bike-shed discussion.
Regards
Takashi Sakamoto
participants (2)
-
Takashi Iwai
-
Takashi Sakamoto