ALSA: control: fix failure to return numerical ID in event data

Commit Message

Takashi Iwai Feb. 8, 2015, 11:09 a.m. UTC

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 <[e-mail]>

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:


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

Comments

Takashi Sakamoto Feb. 8, 2015, 1:08 p.m. UTC | #1

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 <[e-mail]>
> 
> 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

Patch

--- 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);