Re: [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c

[Takashi Iwai <tiwai@suse.de>]

At Sun, 19 Oct 2014 11:38:58 +0200,
Takashi Iwai wrote:
> 
> At Sun, 19 Oct 2014 09:11:26 +0200,
> Daniel Mack wrote:
> > 
> > Out of principles, use strncpy() in favor of strcpy().
> > That is, however, an insignificant detail here.
> > 
> > Signed-off-by: Daniel Mack <daniel@zonque.org>
> 
> Well, blindly doing this isn't optimal, IMO.
> First off, strlcpy() is a better one.  And, in the code you patched,
> we already know all strings to be passed.  That is, if anything is
> over the buffer size, it's a clear bug.  This can be caught by static
> analyzers, or put some debug codes (either for build time or compile
> time) instead of silently trimming the string.

BTW, there is already a nice helper function, snd_ctl_enum_info(), for
the safe enum info setup.  Then the patch would become even more
reducing, something like below.  We can cover many other places in
similar ways.

A further step would be to add a kernel warning when the given string
is too long as an enum item string.  Then we can catch the buggy
driver, too.  I'll cook up the patch.


Takashi

---
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index f119a41ed9a9..dd4d5bdea423 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -441,15 +441,7 @@ static int snd_emu0204_ch_switch_info(struct snd_kcontrol *kcontrol,
 				       "3/4"
 	};
 
-	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
-	uinfo->count = 1;
-	uinfo->value.enumerated.items = 2;
-	if (uinfo->value.enumerated.item > 1)
-		uinfo->value.enumerated.item = 1;
-	strcpy(uinfo->value.enumerated.name,
-		texts[uinfo->value.enumerated.item]);
-
-	return 0;
+	return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
 }
 
 static int snd_emu0204_ch_switch_get(struct snd_kcontrol *kcontrol,
@@ -745,15 +737,7 @@ static int snd_ftu_eff_switch_info(struct snd_kcontrol *kcontrol,
 				       "Echo"
 	};
 
-	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
-	uinfo->count = 1;
-	uinfo->value.enumerated.items = 8;
-	if (uinfo->value.enumerated.item > 7)
-		uinfo->value.enumerated.item = 7;
-	strcpy(uinfo->value.enumerated.name,
-		texts[uinfo->value.enumerated.item]);
-
-	return 0;
+	return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
 }
 
 static int snd_ftu_eff_switch_get(struct snd_kcontrol *kctl,