Hi Takashi,
Thank you for your detailed feedback on the v2 patch. I've prepared a v3 patch series that incorporates your suggestions.
Based on your comments, I've made the following changes:
1. Split the patch into two parts: - Patch 1/2: Adds `scnprintf_append()` to `lib/vsprintf.c` - Patch 2/2: Converts `wavefront.c` to use it 2. Fixed the return value you pointed out 3. Used strnlen() instead of strlen() for safety
I plan to submit this as a two-patch series. However, before I send it, I'd like to confirm a few things:
1. Is this approach (adding the helper to lib/vsprintf.c) what you had in mind? Or would you prefer a different location? 2. Would you recommend sending both patches together, or waiting until patch 1/2 is reviewed and accepted before submitting patch 2/2?
The implementation of scnprintf_append() is shown below: +int scnprintf_append(char *buf, size_t size, const char *fmt, ...) +{ + va_list args; + size_t len; + + len = strnlen(buf, size); + if (len >= size) + return len; + va_start(args, fmt); + len += vscnprintf(buf + len, size - len, fmt, args); + va_end(args); + return len; +}
Thanks again for your guidance.
Best regards, Junrui
________________________________________ From: Takashi Iwai tiwai@suse.de Sent: Tuesday, November 4, 2025 18:01 To: moonafterrain@outlook.com moonafterrain@outlook.com Cc: Jaroslav Kysela perex@perex.cz; Takashi Iwai tiwai@suse.com; linux-sound@vger.kernel.org linux-sound@vger.kernel.org; linux-kernel@vger.kernel.org linux-kernel@vger.kernel.org; stable@vger.kernel.org stable@vger.kernel.org; Yuhao Jiang danisjiang@gmail.com Subject: Re: [PATCH v2] ALSA: wavefront: use scnprintf for longname construction 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@ME2PR01MB...
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