Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper
On Thu, 31 May 2018 21:02:05 +0200,
Andy Shevchenko wrote:
>
> On Thu, May 31, 2018 at 9:59 PM, Takashi Iwai wrote:
> > On Thu, 31 May 2018 20:41:36 +0200,
> > Andy Shevchenko wrote:
> >> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie
> >> wrote:
>
> >> > + j = match_string(known_ctl_names, CONTROL_COUNT,
> >> > ctl->id.name);
> >> > + if (j >= 0) {
> >> > + chip->controls[j] = ctl;
> >> > + ctl->private_free = oxygen_any_ctl_free;
> >> > + }
> >>
> >> It looks to me you may get rid of j completely by utilizing existing err.
> >
> > Well, err isn't ideal as it's referred as the actual index.
> > That is, the line below looks weird to me:
> > chip->controls[err] = ctl;
> >
> > Of course, j isn't the best name, either, but at least, keeping the
> > same variable makes the code conversion logic clearer.
>
> Works for me either way.
> Thanks!
OK, let's take as is.
thanks,
Takashi
Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper
On Thu, May 31, 2018 at 9:59 PM, Takashi Iwai wrote:
> On Thu, 31 May 2018 20:41:36 +0200,
> Andy Shevchenko wrote:
>> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie wrote:
>> > + j = match_string(known_ctl_names, CONTROL_COUNT,
>> > ctl->id.name);
>> > + if (j >= 0) {
>> > + chip->controls[j] = ctl;
>> > + ctl->private_free = oxygen_any_ctl_free;
>> > + }
>>
>> It looks to me you may get rid of j completely by utilizing existing err.
>
> Well, err isn't ideal as it's referred as the actual index.
> That is, the line below looks weird to me:
> chip->controls[err] = ctl;
>
> Of course, j isn't the best name, either, but at least, keeping the
> same variable makes the code conversion logic clearer.
Works for me either way.
Thanks!
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper
On Thu, 31 May 2018 20:41:36 +0200,
Andy Shevchenko wrote:
>
> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie wrote:
> > match_string() returns the index of an array for a matching string,
> > which can be used instead of open coded variant.
>
> Sorry, didn't notice before one thing:
>
> > + j = match_string(known_ctl_names, CONTROL_COUNT,
> > ctl->id.name);
> > + if (j >= 0) {
> > + chip->controls[j] = ctl;
> > + ctl->private_free = oxygen_any_ctl_free;
> > + }
>
> It looks to me you may get rid of j completely by utilizing existing err.
Well, err isn't ideal as it's referred as the actual index.
That is, the line below looks weird to me:
chip->controls[err] = ctl;
Of course, j isn't the best name, either, but at least, keeping the
same variable makes the code conversion logic clearer.
thanks,
Takashi
Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper
On Thu, May 31, 2018 at 9:43 PM, Takashi Iwai wrote: > On Thu, 31 May 2018 20:40:28 +0200, > Andy Shevchenko wrote: >> >> On Thu, May 31, 2018 at 9:39 PM, Takashi Iwai wrote: >> > Applied, thanks. >> >> Is it too late for nitpick? > > Depends :) See my previous mail then. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper
On Thu, 31 May 2018 20:40:28 +0200, Andy Shevchenko wrote: > > On Thu, May 31, 2018 at 9:39 PM, Takashi Iwai wrote: > > On Thu, 31 May 2018 13:11:20 +0200, > > Yisheng Xie wrote: > >> > >> match_string() returns the index of an array for a matching string, > >> which can be used instead of open coded variant. > >> > >> Cc: Clemens Ladisch > >> Cc: Jaroslav Kysela > >> Cc: Takashi Iwai > >> Cc: alsa-de...@alsa-project.org > >> Signed-off-by: Yisheng Xie > > > > Applied, thanks. > > Is it too late for nitpick? Depends :) Takashi
Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper
On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
Sorry, didn't notice before one thing:
> + j = match_string(known_ctl_names, CONTROL_COUNT,
> ctl->id.name);
> + if (j >= 0) {
> + chip->controls[j] = ctl;
> + ctl->private_free = oxygen_any_ctl_free;
> + }
It looks to me you may get rid of j completely by utilizing existing err.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper
On Thu, May 31, 2018 at 9:39 PM, Takashi Iwai wrote: > On Thu, 31 May 2018 13:11:20 +0200, > Yisheng Xie wrote: >> >> match_string() returns the index of an array for a matching string, >> which can be used instead of open coded variant. >> >> Cc: Clemens Ladisch >> Cc: Jaroslav Kysela >> Cc: Takashi Iwai >> Cc: alsa-de...@alsa-project.org >> Signed-off-by: Yisheng Xie > > Applied, thanks. Is it too late for nitpick? -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper
On Thu, 31 May 2018 13:11:20 +0200, Yisheng Xie wrote: > > match_string() returns the index of an array for a matching string, > which can be used instead of open coded variant. > > Cc: Clemens Ladisch > Cc: Jaroslav Kysela > Cc: Takashi Iwai > Cc: alsa-de...@alsa-project.org > Signed-off-by: Yisheng Xie Applied, thanks. Takashi
[PATCH v2 15/21] ALSA: oxygen: use match_string() helper
match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.
Cc: Clemens Ladisch
Cc: Jaroslav Kysela
Cc: Takashi Iwai
Cc: alsa-de...@alsa-project.org
Signed-off-by: Yisheng Xie
---
v2:
- do not change the type of i - per Andy
sound/pci/oxygen/oxygen_mixer.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/sound/pci/oxygen/oxygen_mixer.c b/sound/pci/oxygen/oxygen_mixer.c
index 4ca1266..81af21a 100644
--- a/sound/pci/oxygen/oxygen_mixer.c
+++ b/sound/pci/oxygen/oxygen_mixer.c
@@ -1052,10 +1052,10 @@ static int add_controls(struct oxygen *chip,
[CONTROL_CD_CAPTURE_SWITCH] = "CD Capture Switch",
[CONTROL_AUX_CAPTURE_SWITCH] = "Aux Capture Switch",
};
- unsigned int i, j;
+ unsigned int i;
struct snd_kcontrol_new template;
struct snd_kcontrol *ctl;
- int err;
+ int j, err;
for (i = 0; i < count; ++i) {
template = controls[i];
@@ -1086,11 +1086,11 @@ static int add_controls(struct oxygen *chip,
err = snd_ctl_add(chip->card, ctl);
if (err < 0)
return err;
- for (j = 0; j < CONTROL_COUNT; ++j)
- if (!strcmp(ctl->id.name, known_ctl_names[j])) {
- chip->controls[j] = ctl;
- ctl->private_free = oxygen_any_ctl_free;
- }
+ j = match_string(known_ctl_names, CONTROL_COUNT, ctl->id.name);
+ if (j >= 0) {
+ chip->controls[j] = ctl;
+ ctl->private_free = oxygen_any_ctl_free;
+ }
}
return 0;
}
--
1.7.12.4
