Re: [PATCH v2] ALSA: wavefront: use scnprintf for longname construction

[Takashi Iwai <tiwai@suse.de>]

On Sun, 02 Nov 2025 16:32:39 +0100,
moonafterrain@outlook.com wrote:
> 
> From: Junrui Luo <moonafterrain@outlook.com>
> 
> Replace sprintf() calls with scnprintf() and a new scnprintf_append()
> helper function when constructing card->longname. This improves code
> readability and provides bounds checking for the 80-byte buffer.
> 
> While the current parameter ranges don't cause overflow in practice,
> using safer string functions follows kernel best practices and makes
> the code more maintainable.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
> Changes in v2:
> - Replace sprintf() calls with scnprintf() and a new scnprintf_append()
> - Link to v1: https://lore.kernel.org/all/ME2PR01MB3156CEC4F31F253C9B540FB7AFFDA@ME2PR01MB3156.ausprd01.prod.outlook.com/

Well, my suggestion was that we can apply such conversions once if a
*generic* helper becomes available; that is, propose
scnprintf_append() to be put in include/linux/string.h or whatever (I
guess better in *.c instead of inline), and once if it's accepted, we
can convert the relevant places (there are many, not only
wavefront.c).

BTW:

> +__printf(3, 4) static int scnprintf_append(char *buf, size_t size, const char *fmt, ...)
> +{
> +	va_list args;
> +	size_t len = strlen(buf);
> +
> +	if (len >= size)
> +		return len;
> +	va_start(args, fmt);
> +	len = vscnprintf(buf + len, size - len, fmt, args);
> +	va_end(args);
> +	return len;

The above should be
	len += vscnprintf(buf + len, size - len, fmt, args);
so that it returns the full size of the string.
If it were in user-space, I'd check a negative error code, but the
Linux kernel implementation doesn't return a negative error code, so
far.
I see it's a copy from a code snipped I suggested which already
contained the error :)

Also, it might be safer to use strnlen() instead of strlen() for
avoiding a potential out-of-bound access.


thanks,

Takashi