[PATCH v2] ALSA: hda/generic: Replace strlcat() with strscpy()

[PATCH v2] ALSA: hda/generic: Replace strlcat() with strscpy()

[Ian Bridges]

In preparation for removing the strlcat() API[1], replace its uses in
fill_pcm_stream_name() and get_jack_mode_name(). Both appends become
bounded strscpy() calls at a known position.

In fill_pcm_stream_name() the trim walk itself lands on the append
position. It stops at the trim point when the name has a decoration
to drop, and at the terminating NUL otherwise, so the suffix
overwrites from there and no length bookkeeping is needed.

get_jack_mode_name() measures the label once with strnlen() because
snd_hda_get_pin_label() does not return a length.

The produced strings are unchanged.

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <icb@fastmail.org>
---
v2: land on the append position instead of measuring before every
append. In fill_pcm_stream_name() the trim walk already finishes at
the append position, so the suffix goes down with one bounded
strscpy() and no length bookkeeping, matching the pattern of commit
86dc52c5c96e ("ALSA: usb-audio: simplify mixer control name
handling"). get_jack_mode_name() keeps one strnlen() because
snd_hda_get_pin_label() returns no length. Other shapes were built
and measured for this respin, including the seq_buf form suggested
in the thread. That one moves the fill-only-when-empty guard of
fill_pcm_stream_name() into all three callers, because
seq_buf_init() clears the first byte, and every call site grew.

v1: https://lore.kernel.org/all/alaPxW5S5PT_-kZi@dev/

 sound/hda/codecs/generic.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/hda/codecs/generic.c b/sound/hda/codecs/generic.c
index 660a9f2c0ded..5e23d1345b43 100644
--- a/sound/hda/codecs/generic.c
+++ b/sound/hda/codecs/generic.c
@@ -2712,10 +2712,12 @@ static void get_jack_mode_name(struct hda_codec *codec, hda_nid_t pin,
 			       char *name, size_t name_len)
 {
 	struct hda_gen_spec *spec = codec->spec;
+	size_t used;
 	int idx = 0;
 
 	snd_hda_get_pin_label(codec, pin, &spec->autocfg, name, name_len, &idx);
-	strlcat(name, " Jack Mode", name_len);
+	used = strnlen(name, name_len);
+	strscpy(name + used, " Jack Mode", name_len - used);
 
 	for (; find_kctl_name(codec, name, idx); idx++)
 		;
@@ -5686,16 +5688,15 @@ static void fill_pcm_stream_name(char *str, size_t len, const char *sfx,
 
 	if (*str)
 		return;
+
 	strscpy(str, chip_name, len);
 
-	/* drop non-alnum chars after a space */
-	for (p = strchr(str, ' '); p; p = strchr(p + 1, ' ')) {
-		if (!isalnum(p[1])) {
-			*p = 0;
+	/* find the append position, trimming decorations if present */
+	for (p = str; *p; p++)
+		if (*p == ' ' && !isalnum(p[1]))
 			break;
-		}
-	}
-	strlcat(str, sfx, len);
+
+	strscpy(p, sfx, len - (p - str));
 }
 
 /* copy PCM stream info from @default_str, and override non-NULL entries
-- 
2.47.3

Re: [PATCH v2] ALSA: hda/generic: Replace strlcat() with strscpy()

[Takashi Iwai]

On Wed, 29 Jul 2026 18:06:52 +0200,
Ian Bridges wrote:
> 
> In preparation for removing the strlcat() API[1], replace its uses in
> fill_pcm_stream_name() and get_jack_mode_name(). Both appends become
> bounded strscpy() calls at a known position.
> 
> In fill_pcm_stream_name() the trim walk itself lands on the append
> position. It stops at the trim point when the name has a decoration
> to drop, and at the terminating NUL otherwise, so the suffix
> overwrites from there and no length bookkeeping is needed.
> 
> get_jack_mode_name() measures the label once with strnlen() because
> snd_hda_get_pin_label() does not return a length.
> 
> The produced strings are unchanged.
> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Ian Bridges <icb@fastmail.org>
> ---
> v2: land on the append position instead of measuring before every
> append. In fill_pcm_stream_name() the trim walk already finishes at
> the append position, so the suffix goes down with one bounded
> strscpy() and no length bookkeeping, matching the pattern of commit
> 86dc52c5c96e ("ALSA: usb-audio: simplify mixer control name
> handling"). get_jack_mode_name() keeps one strnlen() because
> snd_hda_get_pin_label() returns no length. Other shapes were built
> and measured for this respin, including the seq_buf form suggested
> in the thread. That one moves the fill-only-when-empty guard of
> fill_pcm_stream_name() into all three callers, because
> seq_buf_init() clears the first byte, and every call site grew.
> 
> v1: https://lore.kernel.org/all/alaPxW5S5PT_-kZi@dev/

Honestly speaking, I still don't like those changes, sorry.

My argument is about neither code correctness nor efficiency.
Instead, it's about that such a open code makes harder to understand
the intention of the code.

The meaning of strlcat() is clear: append a suffix string in a safe
way.  Meanwhile, when you open-code it:

	size_t used;
	....
 	snd_hda_get_pin_label(codec, pin, &spec->autocfg, name, name_len, &idx);
	used = strnlen(name, name_len);
	strscpy(name + used, " Jack Mode", name_len - used);

it's no longer obvious at a first glance, and you'd need to think of
it much longer -- what does this and whether it's really safe.

So, if we must inevitably drop strlcat() function, it's better to
rearrange the caller side beforehand, e.g. replace strlcat() call to
a local function like append_suffix(), in order to make the meaning
of the code clearer.  Then we can replace the code in append_suffix().

// append a suffix string safely; equivalent with strlcat()
static void append_suffix(char *str, const char *suffix, size_t size)
{
	size_t used = strnlen(str, size);
	strscpy(str + used, suffix, size - used);
}

....
	snd_hda_get_pin_label(codec, pin, &spec->autocfg, name, name_len, &idx);
	append_suffix(name, " Jack Mode", name_len);

Since I've been already working on a cleanup of the relevant code, I'm
going to change strlcat() to append_suffix() (but with strlcat()) as a
preparation, too.  Once when we decide to drop strlcat() actually, we
can change the code in that function.


thanks,

Takashi

Re: [PATCH v2] ALSA: hda/generic: Replace strlcat() with strscpy()

[Ian Bridges]

On Thu, Jul 30, 2026 at 11:07:36AM +0200, Takashi Iwai wrote:
> Since I've been already working on a cleanup of the relevant code, I'm
> going to change strlcat() to append_suffix() (but with strlcat()) as a
> preparation, too.  Once when we decide to drop strlcat() actually, we
> can change the code in that function.

Thank you for picking this up. That covers everything the patch was
after, so I'll drop v2. Happy to test or review the cleanup when
it is posted.

Thanks,
Ian