* [PATCH 1/2] ALSA: snd-usb: drop unused varible assigments @ 2014-10-19 7:11 Daniel Mack 2014-10-19 7:11 ` [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c Daniel Mack 2014-10-19 9:36 ` [PATCH 1/2] ALSA: snd-usb: drop unused varible assigments Takashi Iwai 0 siblings, 2 replies; 6+ messages in thread From: Daniel Mack @ 2014-10-19 7:11 UTC (permalink / raw) To: alsa-devel; +Cc: tiwai, Daniel Mack Don't assign 'len' in cases where we don't make use of the returned value. Signed-off-by: Daniel Mack <daniel@zonque.org> --- sound/usb/mixer.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 2e4a9db..63a8adb 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -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"); -- 2.1.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c 2014-10-19 7:11 [PATCH 1/2] ALSA: snd-usb: drop unused varible assigments Daniel Mack @ 2014-10-19 7:11 ` Daniel Mack 2014-10-19 9:38 ` Takashi Iwai 2014-10-19 9:36 ` [PATCH 1/2] ALSA: snd-usb: drop unused varible assigments Takashi Iwai 1 sibling, 1 reply; 6+ messages in thread From: Daniel Mack @ 2014-10-19 7:11 UTC (permalink / raw) To: alsa-devel; +Cc: tiwai, Daniel Mack Out of principles, use strncpy() in favor of strcpy(). That is, however, an insignificant detail here. Signed-off-by: Daniel Mack <daniel@zonque.org> --- sound/usb/mixer_quirks.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index f119a41..f406305 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -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; } -- 2.1.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c 2014-10-19 7:11 ` [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c Daniel Mack @ 2014-10-19 9:38 ` Takashi Iwai 2014-10-20 14:40 ` Takashi Iwai 0 siblings, 1 reply; 6+ messages in thread From: Takashi Iwai @ 2014-10-19 9:38 UTC (permalink / raw) To: Daniel Mack; +Cc: alsa-devel 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. thanks, Takashi > --- > sound/usb/mixer_quirks.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > index f119a41..f406305 100644 > --- a/sound/usb/mixer_quirks.c > +++ b/sound/usb/mixer_quirks.c > @@ -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; > } > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c 2014-10-19 9:38 ` Takashi Iwai @ 2014-10-20 14:40 ` Takashi Iwai 2014-10-20 14:47 ` Daniel Mack 0 siblings, 1 reply; 6+ messages in thread From: Takashi Iwai @ 2014-10-20 14:40 UTC (permalink / raw) To: Daniel Mack; +Cc: alsa-devel 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, ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c 2014-10-20 14:40 ` Takashi Iwai @ 2014-10-20 14:47 ` Daniel Mack 0 siblings, 0 replies; 6+ messages in thread From: Daniel Mack @ 2014-10-20 14:47 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On 10/20/2014 04:40 PM, Takashi Iwai wrote: > 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. Sounds good to me. And yes, we know which values we write to that buffer and that they can't possibly explode it. I was just thinking of the next reader of the code who might be quicker in the review with some obvious guards in place. Thanks, Daniel > > > 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, > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ALSA: snd-usb: drop unused varible assigments 2014-10-19 7:11 [PATCH 1/2] ALSA: snd-usb: drop unused varible assigments Daniel Mack 2014-10-19 7:11 ` [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c Daniel Mack @ 2014-10-19 9:36 ` Takashi Iwai 1 sibling, 0 replies; 6+ messages in thread From: Takashi Iwai @ 2014-10-19 9:36 UTC (permalink / raw) To: Daniel Mack; +Cc: alsa-devel At Sun, 19 Oct 2014 09:11:25 +0200, Daniel Mack wrote: > > Don't assign 'len' in cases where we don't make use of the returned value. > > Signed-off-by: Daniel Mack <daniel@zonque.org> Thanks, applied. Takashi > --- > sound/usb/mixer.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c > index 2e4a9db..63a8adb 100644 > --- a/sound/usb/mixer.c > +++ b/sound/usb/mixer.c > @@ -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"); > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-20 14:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-19 7:11 [PATCH 1/2] ALSA: snd-usb: drop unused varible assigments Daniel Mack 2014-10-19 7:11 ` [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c Daniel Mack 2014-10-19 9:38 ` Takashi Iwai 2014-10-20 14:40 ` Takashi Iwai 2014-10-20 14:47 ` Daniel Mack 2014-10-19 9:36 ` [PATCH 1/2] ALSA: snd-usb: drop unused varible assigments Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.