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

On Wed, 05 Nov 2025 05:28:27 +0100, 222 Summ wrote:

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?

IMO lib/vsprintf.c should be fine.

2. Would you recommend sending both patches together, or waiting until

patch 1/2 is reviewed and accepted before submitting patch 2/2?

You can try patching not only wavefront.c but covering a few others in the series at first for showing that it really makes sense to be a generic helper function. And, submitting the whole might be better to understand what's the use and effect, too.

Note that there can be suggestions for other forms, e.g. there have been some progresses for the continuous string processing, IIRC. So this is merely one example to achieve the goal.

The merit of this way would be its simplicity, though: you can replace the code with a single function call without keeping anything else, and the handling logic is quite straightforward.

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;

+}

This should be with EXPORT_SYMBOL_GPL() (or EXPORT_SYMBOL() is any user is non-GPL).

thanks,

Takashi

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