ALSA: snd-usb: drop unused varible assigments

Out of principles, use strncpy() in favor of strcpy(). That is, however, an insignificant detail here.

Signed-off-by Daniel Mack <[email protected]>

---
 sound/usb/mixer_quirks.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

sound/usb/mixer_quirks.c +3 -2

index f119a41..f406305 100644
@@ -446,8 +446,9 @@ static int snd_emu0204_ch_switch_info(struct snd_kcontrol *kcontrol,
 	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]);
+	strncpy(uinfo->value.enumerated.name,
+		texts[uinfo->value.enumerated.item],
+		sizeof(uinfo->value.enumerated.name) - 1);
 
 	return 0;
 }

Signature · 1 line

2.1.0

At Sun, 19 Oct 2014 09:11:26 +0200,

Quoted 4 lines · Daniel Mack

Out of principles, use strncpy() in favor of strcpy(). That is, however, an insignificant detail here.

Quoted 1 line

Signed-off-by Daniel Mack <[email protected]>

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.

At Sun, 19 Oct 2014 11:38:58 +0200,

Quoted 7 lines · Takashi Iwai

At Sun, 19 Oct 2014 09:11:26 +0200,

Quoted 4 lines · Daniel Mack

Out of principles, use strncpy() in favor of strcpy(). That is, however, an insignificant detail here.

Quoted 8 lines

Quoted 1 line

Signed-off-by Daniel Mack <[email protected]>

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

---

sound/usb/mixer_quirks.c +2 -18

index f119a41ed9a9..dd4d5bdea423 100644
@@ -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,

Don't assign 'len' in cases where we don't make use of the returned value.

Signed-off-by Daniel Mack <[email protected]>

---
 sound/usb/mixer.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

sound/usb/mixer.c +4 -5

index 2e4a9db..63a8adb 100644
@@ -1290,9 +1290,8 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 						    kctl->id.name,
 						    sizeof(kctl->id.name), 1);
 			if (!len)
-				len = snprintf(kctl->id.name,
-					       sizeof(kctl->id.name),
-					       "Feature %d", unitid);
+				snprintf(kctl->id.name, sizeof(kctl->id.name),
+					 "Feature %d", unitid);
 		}
 
 		if (!mapped_name)

@@ -1305,9 +1304,9 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 		 */
 		if (!mapped_name && !(state->oterm.type >> 16)) {
 			if ((state->oterm.type & 0xff00) == 0x0100)
-				len = append_ctl_name(kctl, " Capture");
+				append_ctl_name(kctl, " Capture");
 			else
-				len = append_ctl_name(kctl, " Playback");
+				append_ctl_name(kctl, " Playback");
 		}
 		append_ctl_name(kctl, control == UAC_FU_MUTE ?
 				" Switch" : " Volume");

Signature · 1 line

2.1.0