[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