[PATCH] ASoC: Fix long name of control for dapm_switch

[PATCH] ASoC: Fix long name of control for dapm_switch

[Joonyoung Shim]

The length of long control name doesn't include the length of widget
name at present when widget->id is snd_soc_dapm_switch, so the long
control name of dapm_switch is shortened.
To avoid this problem, the long control name only for dapm_mixer_named_ctl
must have simply the kcontrol name.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 sound/soc/soc-dapm.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 4b8dbbf..7a1fed1 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -331,26 +331,23 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
                         * for dapm_mixer_named_ctl this is simply the
                         * kcontrol name.
                         */
-                       name_len = strlen(w->kcontrols[i].name) + 1;
-                       if (w->id == snd_soc_dapm_mixer)
-                               name_len += 1 + strlen(w->name);
+                       if (w->id == snd_soc_dapm_mixer_named_ctl)
+                               name_len = strlen(w->kcontrols[i].name) + 1;
+                       else
+                               name_len = 2 + strlen(w->name)
+                                       + strlen(w->kcontrols[i].name);

                        path->long_name = kmalloc(name_len, GFP_KERNEL);

                        if (path->long_name == NULL)
                                return -ENOMEM;

-                       switch (w->id) {
-                       case snd_soc_dapm_mixer:
-                       default:
-                               snprintf(path->long_name, name_len, "%s %s",
-                                        w->name, w->kcontrols[i].name);
-                       break;
-                       case snd_soc_dapm_mixer_named_ctl:
+                       if (w->id == snd_soc_dapm_mixer_named_ctl)
                                snprintf(path->long_name, name_len, "%s",
                                         w->kcontrols[i].name);
-                       break;
-                       }
+                       else
+                               snprintf(path->long_name, name_len, "%s %s",
+                                        w->name, w->kcontrols[i].name);

                        path->long_name[name_len - 1] = '\0';

-- 
1.5.6.3


-- 
- Joonyoung Shim

Re: [PATCH] ASoC: Fix long name of control for dapm_switch

[Mark Brown]

On Wed, Mar 04, 2009 at 10:47:09AM +0900, Joonyoung Shim wrote:
> The length of long control name doesn't include the length of widget
> name at present when widget->id is snd_soc_dapm_switch, so the long
> control name of dapm_switch is shortened.
> To avoid this problem, the long control name only for dapm_mixer_named_ctl
> must have simply the kcontrol name.

This is a good catch - thanks!

Unfortunately your patch doesn't apply against the current topic/asoc
branch of Takashi's repository, I've implemented a slightly different
version of it.

> -                       name_len = strlen(w->kcontrols[i].name) + 1;
> -                       if (w->id == snd_soc_dapm_mixer)
> -                               name_len += 1 + strlen(w->name);
> +                       if (w->id == snd_soc_dapm_mixer_named_ctl)
> +                               name_len = strlen(w->kcontrols[i].name) + 1;
> +                       else
> +                               name_len = 2 + strlen(w->name)
> +                                       + strlen(w->kcontrols[i].name);

Here I just changed the test to != named_ctl.

> 
> -                       switch (w->id) {
> -                       case snd_soc_dapm_mixer:
> -                       default:

Your chanegs here appear to only flip to an if/else from a switch
statement?  Please mention things like this in your commit logs (or
split them into separate patches) - it makes your changes easier to
review.

Re: [PATCH] ASoC: Fix long name of control for dapm_switch

[Joonyoung Shim]

> Unfortunately your patch doesn't apply against the current topic/asoc
> branch of Takashi's repository, I've implemented a slightly different
> version of it.
>
>> -                       name_len = strlen(w->kcontrols[i].name) + 1;
>> -                       if (w->id == snd_soc_dapm_mixer)
>> -                               name_len += 1 + strlen(w->name);
>> +                       if (w->id == snd_soc_dapm_mixer_named_ctl)
>> +                               name_len = strlen(w->kcontrols[i].name) + 1;
>> +                       else
>> +                               name_len = 2 + strlen(w->name)
>> +                                       + strlen(w->kcontrols[i].name);
>
> Here I just changed the test to != named_ctl.

ok, i will resend the patch.

>
>>
>> -                       switch (w->id) {
>> -                       case snd_soc_dapm_mixer:
>> -                       default:
>
> Your chanegs here appear to only flip to an if/else from a switch
> statement?  Please mention things like this in your commit logs (or
> split them into separate patches) - it makes your changes easier to
> review.

I think that a switch statement is used unnecessarily because this part has
only to check whether w->id is named_ctl.
ok, i will send them into splited patches.
Thanks.


-- 
- Joonyoung Shim

Re: [PATCH] ASoC: Fix long name of control for dapm_switch

[Joonyoung Shim]

> ok, i will resend the patch.
>
>>
>>>
>>> -                       switch (w->id) {
>>> -                       case snd_soc_dapm_mixer:
>>> -                       default:
>>
>> Your chanegs here appear to only flip to an if/else from a switch
>> statement?  Please mention things like this in your commit logs (or
>> split them into separate patches) - it makes your changes easier to
>> review.
>
> I think that a switch statement is used unnecessarily because this part has
> only to check whether w->id is named_ctl.
> ok, i will send them into splited patches.
> Thanks.

I found your commit at for-2.6.30 branch of your repository.
Thanks for your fixes.

-- 
- Joonyoung Shim