From: Takashi Iwai <tiwai@suse.de>
To: "Mahad Ibrahim" <mahad.ibrahim.dev@gmail.com>
Cc: "Takashi Iwai" <tiwai@suse.de>, "Takashi Iwai" <tiwai@suse.com>,
"Jaroslav Kysela" <perex@perex.cz>, "Kees Cook" <kees@kernel.org>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
<linux-sound@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/
Date: Fri, 07 Aug 2026 18:03:58 +0200 [thread overview]
Message-ID: <878q6hc3yp.wl-tiwai@suse.de> (raw)
In-Reply-To: <DKITQMNPN2I7.CH00E8CSZMXI@gmail.com>
On Fri, 07 Aug 2026 17:41:36 +0200,
Mahad Ibrahim wrote:
>
> On Fri Aug 7, 2026 at 5:15 PM PKT, Takashi Iwai wrote:
> > Honestly speaking, I'm against those conversions.
> > Why do we have to open-code at each place with strlen()+strscpy()?
> > It's just harder to read than strlcat(), even more error-prone.
> >
> > If an alternative is something like this, we really should reconsider.
>
> Thank you for the quick response and feedback.
>
> You are right that strlen() + strscpy() is tedious and annoying to read.
>
> sound/ already has helpers that do this, but each is local to one file
> with its own signature:
>
> safe_append_string() sound/core/ump.c
> append_ctl_name() sound/usb/mixer.c
> hda_append_suffix() sound/hda/common/hda_local.h
>
> Each of these functions practice the same string append technique
> however to slightly different effect. safe_append_string uses
> safe_copy_string() whose function body is above it in the same file.
> safe_copy_string() performs analogous to strscpy() however adds a
> filter which drops non-printable ASCII characters during the copy phase.
>
> append_ctl_name() is simply an strlcat wrapper which when transitioned
> would result in the same strlen() + strscpy(). Only separating feature
> is that it returns the length of characters that would have been
> written (not necessarily the actual amount). However none of the callers
> use its return functionality.
>
> hda_append_suffix() is a verbatim copy of strlen() + strscpy().
>
> A solution I would propose is that all these functions which do the same
> thing, aside from safe_append_string, could be moved where they are
> accessible globally across sound/. This would remove the redundant need
> to use strlen() + strscpy() in replacement for strlcat() and would unify
> the sub-system under a single string append API.
>
> safe_append_string is only called once in the entire sub-system, and could
> be replaced either within the function with the unified string
> append function, and a separate filterer replacing the safe_copy_string
> function or removed all together and managed inline within the single
> caller. However this function would require a more involved removal as the
> internal safe_copy_string is called twice; it is called once in
> safe_append_string(), and in sound/core/ump.c for a wrapper function
> ump_set_rawmidi_name().
>
> An argument against this suggestion is that it would confine a string
> append helper to a single sub-system, while the rest of the kernel uses
> something else.
>
> Additionally patch 1/7 shouldn't have open-coded anything at all.
> safe_append_string() was already a few hundred lines above the site I
> touched, and I should have used it.
>
> This was based on https://github.com/KSPP/linux/issues/370, which I
> should have linked in the cover letter.
>
> Thank you for your time.
>
> Best regards,
> Mahad Ibrahim
Well, that leads to a basic question: why do we have to drop strlcat()
if most of callers would just need the equivalent function.
If strlcat() were super-dangerous, it's understandable to drop. But,
it's not, and issues discussed in the github are minor and something
that can be addressed in strlcat() implementation; that is, can't we
rather re-implement strlcat() in a safer way, instead of killing it?
Sure, there are code calling strlcat() that could be optimized better.
They can be cleaned up. But it alone can't be a reason that strlcat()
must die without mercy.
thanks,
Takashi
next prev parent reply other threads:[~2026-08-07 16:04 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-07 11:41 [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/ Mahad Ibrahim
2026-08-07 11:41 ` [PATCH 1/7] ALSA: ump: replace strlcat() with strscpy() Mahad Ibrahim
2026-08-07 11:41 ` [PATCH 2/7] ALSA: ac97: replace strlcat() with scnprintf() Mahad Ibrahim
2026-08-07 11:41 ` [PATCH 3/7] ALSA: cmipci: replace strlcat() with strscpy() Mahad Ibrahim
2026-08-07 11:41 ` [PATCH 4/7] ALSA: caiaq: " Mahad Ibrahim
2026-08-07 11:41 ` [PATCH 5/7] ALSA: usb-audio: replace strlcat() with append_ctl_name() Mahad Ibrahim
2026-08-07 11:41 ` [PATCH 6/7] ALSA: hiface: replace strlcat() with scnprintf() Mahad Ibrahim
2026-08-07 11:41 ` [PATCH 7/7] ALSA: usb-audio: replace strlcat() in longname construction Mahad Ibrahim
2026-08-07 12:15 ` [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/ Takashi Iwai
2026-08-07 13:04 ` David Laight
2026-08-07 15:41 ` Mahad Ibrahim
2026-08-07 16:03 ` Takashi Iwai [this message]
2026-08-07 21:46 ` Kees Cook
2026-08-08 13:04 ` David Laight
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878q6hc3yp.wl-tiwai@suse.de \
--to=tiwai@suse.de \
--cc=andriy.shevchenko@linux.intel.com \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=mahad.ibrahim.dev@gmail.com \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.