[PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Mariia Nikitash]

In preparation for removing the deprecated strlcat() API[1],
replace the strscpy() and strlcat() calls used to construct
V4L2 card names in the TEA5777 and TEA575x radio drivers.

seq_buf tracks the current write position and remaining capacity
across appends, avoiding repeated scans of the destination buffer.
This preserves the resulting strings, including truncation behavior.

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Mariia Nikitash <nikitash.mariiaw@gmail.com>
---
 drivers/media/radio/radio-tea5777.c | 10 ++++++++--
 drivers/media/radio/tea575x.c       | 10 ++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/media/radio/radio-tea5777.c b/drivers/media/radio/radio-tea5777.c
index fb9de7bbcd19..2b1c7f6ddd60 100644
--- a/drivers/media/radio/radio-tea5777.c
+++ b/drivers/media/radio/radio-tea5777.c
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/sched.h>
+#include <linux/seq_buf.h>
 #include <linux/slab.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-dev.h>
@@ -255,10 +256,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
 					struct v4l2_capability *v)
 {
 	struct radio_tea5777 *tea = video_drvdata(file);
+	struct seq_buf s;
 
 	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
-	strscpy(v->card, tea->card, sizeof(v->card));
-	strlcat(v->card, " TEA5777", sizeof(v->card));
+
+	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
+	seq_buf_puts(&s, (const char *)tea->card);
+	seq_buf_puts(&s, " TEA5777");
+	seq_buf_terminate(&s);
+
 	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
 	return 0;
 }
diff --git a/drivers/media/radio/tea575x.c b/drivers/media/radio/tea575x.c
index c37315226c42..e49228eb7c46 100644
--- a/drivers/media/radio/tea575x.c
+++ b/drivers/media/radio/tea575x.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/seq_buf.h>
 #include <linux/sched.h>
 #include <asm/io.h>
 #include <media/v4l2-device.h>
@@ -221,10 +222,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
 					struct v4l2_capability *v)
 {
 	struct snd_tea575x *tea = video_drvdata(file);
+	struct seq_buf s;
 
 	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
-	strscpy(v->card, tea->card, sizeof(v->card));
-	strlcat(v->card, tea->tea5759 ? " TEA5759" : " TEA5757", sizeof(v->card));
+
+	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
+	seq_buf_puts(&s, (const char *)tea->card);
+	seq_buf_puts(&s, tea->tea5759 ? " TEA5759" : " TEA5757");
+	seq_buf_terminate(&s);
+
 	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
 	return 0;
 }
-- 
2.43.0

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3144 bytes --]

Hi Mariia,

> Date: 2026-11-05 23:46:39-0800
> From: Mariia Nikitash <nikitash.mariiaw@gmail.com>
>
> In preparation for removing the deprecated strlcat() API[1],
> replace the strscpy() and strlcat() calls used to construct
> V4L2 card names in the TEA5777 and TEA575x radio drivers.
> 
> seq_buf tracks the current write position and remaining capacity
> across appends, avoiding repeated scans of the destination buffer.
> This preserves the resulting strings, including truncation behavior.
> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Mariia Nikitash <nikitash.mariiaw@gmail.com>
> ---
>  drivers/media/radio/radio-tea5777.c | 10 ++++++++--
>  drivers/media/radio/tea575x.c       | 10 ++++++++--
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/radio/radio-tea5777.c b/drivers/media/radio/radio-tea5777.c
> index fb9de7bbcd19..2b1c7f6ddd60 100644
> --- a/drivers/media/radio/radio-tea5777.c
> +++ b/drivers/media/radio/radio-tea5777.c
> @@ -13,6 +13,7 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/sched.h>
> +#include <linux/seq_buf.h>
>  #include <linux/slab.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-dev.h>
> @@ -255,10 +256,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  					struct v4l2_capability *v)
>  {
>  	struct radio_tea5777 *tea = video_drvdata(file);
> +	struct seq_buf s;
>  
>  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> -	strscpy(v->card, tea->card, sizeof(v->card));
> -	strlcat(v->card, " TEA5777", sizeof(v->card));
> +
> +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> +	seq_buf_puts(&s, (const char *)tea->card);
> +	seq_buf_puts(&s, " TEA5777");
> +	seq_buf_terminate(&s);
> +

Why are the casts needed?


Have a lovely day!
Alex

>  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
>  	return 0;
>  }
> diff --git a/drivers/media/radio/tea575x.c b/drivers/media/radio/tea575x.c
> index c37315226c42..e49228eb7c46 100644
> --- a/drivers/media/radio/tea575x.c
> +++ b/drivers/media/radio/tea575x.c
> @@ -9,6 +9,7 @@
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> +#include <linux/seq_buf.h>
>  #include <linux/sched.h>
>  #include <asm/io.h>
>  #include <media/v4l2-device.h>
> @@ -221,10 +222,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  					struct v4l2_capability *v)
>  {
>  	struct snd_tea575x *tea = video_drvdata(file);
> +	struct seq_buf s;
>  
>  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> -	strscpy(v->card, tea->card, sizeof(v->card));
> -	strlcat(v->card, tea->tea5759 ? " TEA5759" : " TEA5757", sizeof(v->card));
> +
> +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> +	seq_buf_puts(&s, (const char *)tea->card);
> +	seq_buf_puts(&s, tea->tea5759 ? " TEA5759" : " TEA5757");
> +	seq_buf_terminate(&s);
> +
>  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
>  	return 0;
>  }
> -- 
> 2.43.0
> 
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Mariia Nikitash]

On Fri, Nov 6, 2026 at 2:39 AM Alejandro Colomar
<alx+linux-hardening@kernel.org> wrote:
>
> Hi Mariia,
>
> > Date: 2026-11-05 23:46:39-0800
> > From: Mariia Nikitash <nikitash.mariiaw@gmail.com>
> >
> > In preparation for removing the deprecated strlcat() API[1],
> > replace the strscpy() and strlcat() calls used to construct
> > V4L2 card names in the TEA5777 and TEA575x radio drivers.
> >
> > seq_buf tracks the current write position and remaining capacity
> > across appends, avoiding repeated scans of the destination buffer.
> > This preserves the resulting strings, including truncation behavior.
> >
> > Link: https://github.com/KSPP/linux/issues/370 [1]
> > Signed-off-by: Mariia Nikitash <nikitash.mariiaw@gmail.com>
> > ---
> >  drivers/media/radio/radio-tea5777.c | 10 ++++++++--
> >  drivers/media/radio/tea575x.c       | 10 ++++++++--
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/radio/radio-tea5777.c b/drivers/media/radio/radio-tea5777.c
> > index fb9de7bbcd19..2b1c7f6ddd60 100644
> > --- a/drivers/media/radio/radio-tea5777.c
> > +++ b/drivers/media/radio/radio-tea5777.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/init.h>
> >  #include <linux/module.h>
> >  #include <linux/sched.h>
> > +#include <linux/seq_buf.h>
> >  #include <linux/slab.h>
> >  #include <media/v4l2-device.h>
> >  #include <media/v4l2-dev.h>
> > @@ -255,10 +256,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
> >                                       struct v4l2_capability *v)
> >  {
> >       struct radio_tea5777 *tea = video_drvdata(file);
> > +     struct seq_buf s;
> >
> >       strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > -     strscpy(v->card, tea->card, sizeof(v->card));
> > -     strlcat(v->card, " TEA5777", sizeof(v->card));
> > +
> > +     seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > +     seq_buf_puts(&s, (const char *)tea->card);
> > +     seq_buf_puts(&s, " TEA5777");
> > +     seq_buf_terminate(&s);
> > +
>
> Why are the casts needed?

Hi Alex,

I added the casts to make conversions from u8/__u8 to
character-pointer types expected by the seq_buf helpers explicit.

I'm also considering simplifying these two-part string constructions
using scnprintf() instead. Would this approach be preferred?



> Have a lovely day!
> Alex

Thanks,
Mariia
>
> >       strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
> >       return 0;
> >  }
> > diff --git a/drivers/media/radio/tea575x.c b/drivers/media/radio/tea575x.c
> > index c37315226c42..e49228eb7c46 100644
> > --- a/drivers/media/radio/tea575x.c
> > +++ b/drivers/media/radio/tea575x.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/module.h>
> >  #include <linux/init.h>
> >  #include <linux/slab.h>
> > +#include <linux/seq_buf.h>
> >  #include <linux/sched.h>
> >  #include <asm/io.h>
> >  #include <media/v4l2-device.h>
> > @@ -221,10 +222,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
> >                                       struct v4l2_capability *v)
> >  {
> >       struct snd_tea575x *tea = video_drvdata(file);
> > +     struct seq_buf s;
> >
> >       strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > -     strscpy(v->card, tea->card, sizeof(v->card));
> > -     strlcat(v->card, tea->tea5759 ? " TEA5759" : " TEA5757", sizeof(v->card));
> > +
> > +     seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > +     seq_buf_puts(&s, (const char *)tea->card);
> > +     seq_buf_puts(&s, tea->tea5759 ? " TEA5759" : " TEA5757");
> > +     seq_buf_terminate(&s);
> > +
> >       strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
> >       return 0;
> >  }
> > --
> > 2.43.0
> >
> >
>
> --
> <https://www.alejandro-colomar.es>

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1482 bytes --]

Hi Mariia,

> Date: 2026-11-06 20:31:53-0800
> From: Mariia Nikitash <nikitash.mariiaw@gmail.com>
>
[...]
> > > @@ -255,10 +256,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
> > >                                       struct v4l2_capability *v)
> > >  {
> > >       struct radio_tea5777 *tea = video_drvdata(file);
> > > +     struct seq_buf s;
> > >
> > >       strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > > -     strscpy(v->card, tea->card, sizeof(v->card));
> > > -     strlcat(v->card, " TEA5777", sizeof(v->card));
> > > +
> > > +     seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > > +     seq_buf_puts(&s, (const char *)tea->card);
> > > +     seq_buf_puts(&s, " TEA5777");
> > > +     seq_buf_terminate(&s);
> > > +
> >
> > Why are the casts needed?
> 
> Hi Alex,
> 
> I added the casts to make conversions from u8/__u8 to
> character-pointer types expected by the seq_buf helpers explicit.

I was surprised that it wasn't needed before.  In theory, strscpy()
should be expecting the same types, so if it wasn't needed before, it
shouldn't be needed now (and conversely, if it is needed now, it would
have been needed before).

> 
> I'm also considering simplifying these two-part string constructions
> using scnprintf() instead. Would this approach be preferred?

I certainly prefer scnprintf() over seq_buf.  :)


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[David Laight]

On Thu,  5 Nov 2026 23:46:39 -0800
Mariia Nikitash <nikitash.mariiaw@gmail.com> wrote:

> In preparation for removing the deprecated strlcat() API[1],
> replace the strscpy() and strlcat() calls used to construct
> V4L2 card names in the TEA5777 and TEA575x radio drivers.
> 
> seq_buf tracks the current write position and remaining capacity
> across appends, avoiding repeated scans of the destination buffer.
> This preserves the resulting strings, including truncation behavior.

seq_buf is just OTT.
Using to replace a single strcat() is just silly.
There is no point doing these patches until there is a sensible alternative.

> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Mariia Nikitash <nikitash.mariiaw@gmail.com>
> ---
>  drivers/media/radio/radio-tea5777.c | 10 ++++++++--
>  drivers/media/radio/tea575x.c       | 10 ++++++++--
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/radio/radio-tea5777.c b/drivers/media/radio/radio-tea5777.c
> index fb9de7bbcd19..2b1c7f6ddd60 100644
> --- a/drivers/media/radio/radio-tea5777.c
> +++ b/drivers/media/radio/radio-tea5777.c
> @@ -13,6 +13,7 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/sched.h>
> +#include <linux/seq_buf.h>
>  #include <linux/slab.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-dev.h>
> @@ -255,10 +256,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  					struct v4l2_capability *v)
>  {
>  	struct radio_tea5777 *tea = video_drvdata(file);
> +	struct seq_buf s;
>  
>  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> -	strscpy(v->card, tea->card, sizeof(v->card));
> -	strlcat(v->card, " TEA5777", sizeof(v->card));

What is wrong with (and almost OTT):
#define TEA5777 " TEA5777"
	BUILD_BUG_ON(sizeof(TEA5777) <= sizeof(v->card))

	len = strscpy(v->card, tea->card, sizeof(v->card) - sizeof(TEA5777) + 1);
	if (len < 0)
		goto fail;
	memcpy(v->card + len, TEA5777, sizeof(TEA5777);

What might be useful is a helper that would let you write:
	len = strscpy(v->card, tea->card);
	len = strscpy_at(v->card, len, " TEA5777" [, implied_length]);
	if (len < 0)
		goto fail;

David

> +
> +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> +	seq_buf_puts(&s, (const char *)tea->card);
> +	seq_buf_puts(&s, " TEA5777");
> +	seq_buf_terminate(&s);
> +
>  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
>  	return 0;
>  }
> diff --git a/drivers/media/radio/tea575x.c b/drivers/media/radio/tea575x.c
> index c37315226c42..e49228eb7c46 100644
> --- a/drivers/media/radio/tea575x.c
> +++ b/drivers/media/radio/tea575x.c
> @@ -9,6 +9,7 @@
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> +#include <linux/seq_buf.h>
>  #include <linux/sched.h>
>  #include <asm/io.h>
>  #include <media/v4l2-device.h>
> @@ -221,10 +222,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  					struct v4l2_capability *v)
>  {
>  	struct snd_tea575x *tea = video_drvdata(file);
> +	struct seq_buf s;
>  
>  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> -	strscpy(v->card, tea->card, sizeof(v->card));
> -	strlcat(v->card, tea->tea5759 ? " TEA5759" : " TEA5757", sizeof(v->card));
> +
> +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> +	seq_buf_puts(&s, (const char *)tea->card);
> +	seq_buf_puts(&s, tea->tea5759 ? " TEA5759" : " TEA5757");
> +	seq_buf_terminate(&s);
> +
>  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
>  	return 0;
>  }

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3549 bytes --]

Hi David,

> Date: 2026-11-08 21:02:32+0000
> From: David Laight <david.laight.linux@gmail.com>
>
[...]
> 
> seq_buf is just OTT.
> Using to replace a single strcat() is just silly.
> There is no point doing these patches until there is a sensible alternative.

Agree.

[...]
> >  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > -	strscpy(v->card, tea->card, sizeof(v->card));
> > -	strlcat(v->card, " TEA5777", sizeof(v->card));
> 
> What is wrong with (and almost OTT):
> #define TEA5777 " TEA5777"
> 	BUILD_BUG_ON(sizeof(TEA5777) <= sizeof(v->card))
> 
> 	len = strscpy(v->card, tea->card, sizeof(v->card) - sizeof(TEA5777) + 1);

I can answer this one: bugs (particularly, off by one) are quite prone
in such code.

From the Authors of strlcat(3), you can look at OpenBSD's manual page:

	Since it is known how many characters were copied the first
	time, things can be sped up a bit by using a copy instead of an
	append:

		char *dir, *file, pname[PATH_MAX];
		size_t n;

		...

		n = strlcpy(pname, dir, sizeof(pname));
		if (n >= sizeof(pname))
			goto toolong;
		if (strlcpy(pname + n, file, sizeof(pname) - n) >= sizeof(pname) - n)
			goto toolong;

	However, one may question the validity of such optimizations, as
	they defeat the whole purpose of strlcpy() and strlcat().  As a
	matter of fact, the first version of this manual page got it
	wrong.

> 	if (len < 0)
> 		goto fail;
> 	memcpy(v->card + len, TEA5777, sizeof(TEA5777);
> 
> What might be useful is a helper that would let you write:
> 	len = strscpy(v->card, tea->card);
> 	len = strscpy_at(v->card, len, " TEA5777" [, implied_length]);
> 	if (len < 0)
> 		goto fail;

Indeed, as said in other threads, I have such an API:

	p = v->card;
	e = ARRAY_END(v->card);
	p = stpecpy(p, e, tea->card);
	p = stpecpy(p, e, " TEA5777");
	if (p == NULL)
		goto fail;

Linus didn't like my function name.  We could indeed call it strscpy_at().


Have a lovely night!
Alex

> 
> David
> 
> > +
> > +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > +	seq_buf_puts(&s, (const char *)tea->card);
> > +	seq_buf_puts(&s, " TEA5777");
> > +	seq_buf_terminate(&s);
> > +
> >  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
> >  	return 0;
> >  }
> > diff --git a/drivers/media/radio/tea575x.c b/drivers/media/radio/tea575x.c
> > index c37315226c42..e49228eb7c46 100644
> > --- a/drivers/media/radio/tea575x.c
> > +++ b/drivers/media/radio/tea575x.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/module.h>
> >  #include <linux/init.h>
> >  #include <linux/slab.h>
> > +#include <linux/seq_buf.h>
> >  #include <linux/sched.h>
> >  #include <asm/io.h>
> >  #include <media/v4l2-device.h>
> > @@ -221,10 +222,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
> >  					struct v4l2_capability *v)
> >  {
> >  	struct snd_tea575x *tea = video_drvdata(file);
> > +	struct seq_buf s;
> >  
> >  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > -	strscpy(v->card, tea->card, sizeof(v->card));
> > -	strlcat(v->card, tea->tea5759 ? " TEA5759" : " TEA5757", sizeof(v->card));
> > +
> > +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > +	seq_buf_puts(&s, (const char *)tea->card);
> > +	seq_buf_puts(&s, tea->tea5759 ? " TEA5759" : " TEA5757");
> > +	seq_buf_terminate(&s);
> > +
> >  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
> >  	return 0;
> >  }
> 
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Hans Verkuil]

On 06/11/2026 08:46, Mariia Nikitash wrote:
> In preparation for removing the deprecated strlcat() API[1],
> replace the strscpy() and strlcat() calls used to construct
> V4L2 card names in the TEA5777 and TEA575x radio drivers.
> 
> seq_buf tracks the current write position and remaining capacity
> across appends, avoiding repeated scans of the destination buffer.
> This preserves the resulting strings, including truncation behavior.
> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Mariia Nikitash <nikitash.mariiaw@gmail.com>

Nacked-by: Hans Verkuil <hverkuil+cisco@kernel.org>

This is silly. Just use scnprintf().

Regards,

	Hans

> ---
>  drivers/media/radio/radio-tea5777.c | 10 ++++++++--
>  drivers/media/radio/tea575x.c       | 10 ++++++++--
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/radio/radio-tea5777.c b/drivers/media/radio/radio-tea5777.c
> index fb9de7bbcd19..2b1c7f6ddd60 100644
> --- a/drivers/media/radio/radio-tea5777.c
> +++ b/drivers/media/radio/radio-tea5777.c
> @@ -13,6 +13,7 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/sched.h>
> +#include <linux/seq_buf.h>
>  #include <linux/slab.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-dev.h>
> @@ -255,10 +256,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  					struct v4l2_capability *v)
>  {
>  	struct radio_tea5777 *tea = video_drvdata(file);
> +	struct seq_buf s;
>  
>  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> -	strscpy(v->card, tea->card, sizeof(v->card));
> -	strlcat(v->card, " TEA5777", sizeof(v->card));
> +
> +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> +	seq_buf_puts(&s, (const char *)tea->card);
> +	seq_buf_puts(&s, " TEA5777");
> +	seq_buf_terminate(&s);
> +
>  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
>  	return 0;
>  }
> diff --git a/drivers/media/radio/tea575x.c b/drivers/media/radio/tea575x.c
> index c37315226c42..e49228eb7c46 100644
> --- a/drivers/media/radio/tea575x.c
> +++ b/drivers/media/radio/tea575x.c
> @@ -9,6 +9,7 @@
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> +#include <linux/seq_buf.h>
>  #include <linux/sched.h>
>  #include <asm/io.h>
>  #include <media/v4l2-device.h>
> @@ -221,10 +222,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  					struct v4l2_capability *v)
>  {
>  	struct snd_tea575x *tea = video_drvdata(file);
> +	struct seq_buf s;
>  
>  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> -	strscpy(v->card, tea->card, sizeof(v->card));
> -	strlcat(v->card, tea->tea5759 ? " TEA5759" : " TEA5757", sizeof(v->card));
> +
> +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> +	seq_buf_puts(&s, (const char *)tea->card);
> +	seq_buf_puts(&s, tea->tea5759 ? " TEA5759" : " TEA5757");
> +	seq_buf_terminate(&s);
> +
>  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
>  	return 0;
>  }

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[David Laight]

On Tue, 10 Nov 2026 11:08:30 +0100
Hans Verkuil <hverkuil+cisco@kernel.org> wrote:

> On 06/11/2026 08:46, Mariia Nikitash wrote:
> > In preparation for removing the deprecated strlcat() API[1],
> > replace the strscpy() and strlcat() calls used to construct
> > V4L2 card names in the TEA5777 and TEA575x radio drivers.
> > 
> > seq_buf tracks the current write position and remaining capacity
> > across appends, avoiding repeated scans of the destination buffer.
> > This preserves the resulting strings, including truncation behavior.
> > 
> > Link: https://github.com/KSPP/linux/issues/370 [1]
> > Signed-off-by: Mariia Nikitash <nikitash.mariiaw@gmail.com>  
> 
> Nacked-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> 
> This is silly. Just use scnprintf().

Or wait for someone to add a call that just does what is needed.
In this case the first stscpy() returns the length (and it is often
known).
So you could have:
	len = strscpy(buf, tea->card[, buflen]);
	len = strscpy_at(buf, len, " TEA5777"[, buflen]);
or:
	len = strscat(buf, buflen, tea->card, " TEA5777", ....);
or some variant that will do a strnlen(buf, buflen) to get the offset.

But it really is best to leave the strlcat() calls in there for now.

David

> 
> Regards,
> 
> 	Hans
> 
> > ---
> >  drivers/media/radio/radio-tea5777.c | 10 ++++++++--
> >  drivers/media/radio/tea575x.c       | 10 ++++++++--
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/radio/radio-tea5777.c b/drivers/media/radio/radio-tea5777.c
> > index fb9de7bbcd19..2b1c7f6ddd60 100644
> > --- a/drivers/media/radio/radio-tea5777.c
> > +++ b/drivers/media/radio/radio-tea5777.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/init.h>
> >  #include <linux/module.h>
> >  #include <linux/sched.h>
> > +#include <linux/seq_buf.h>
> >  #include <linux/slab.h>
> >  #include <media/v4l2-device.h>
> >  #include <media/v4l2-dev.h>
> > @@ -255,10 +256,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
> >  					struct v4l2_capability *v)
> >  {
> >  	struct radio_tea5777 *tea = video_drvdata(file);
> > +	struct seq_buf s;
> >  
> >  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > -	strscpy(v->card, tea->card, sizeof(v->card));
> > -	strlcat(v->card, " TEA5777", sizeof(v->card));
> > +
> > +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > +	seq_buf_puts(&s, (const char *)tea->card);
> > +	seq_buf_puts(&s, " TEA5777");
> > +	seq_buf_terminate(&s);
> > +
> >  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
> >  	return 0;
> >  }
> > diff --git a/drivers/media/radio/tea575x.c b/drivers/media/radio/tea575x.c
> > index c37315226c42..e49228eb7c46 100644
> > --- a/drivers/media/radio/tea575x.c
> > +++ b/drivers/media/radio/tea575x.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/module.h>
> >  #include <linux/init.h>
> >  #include <linux/slab.h>
> > +#include <linux/seq_buf.h>
> >  #include <linux/sched.h>
> >  #include <asm/io.h>
> >  #include <media/v4l2-device.h>
> > @@ -221,10 +222,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
> >  					struct v4l2_capability *v)
> >  {
> >  	struct snd_tea575x *tea = video_drvdata(file);
> > +	struct seq_buf s;
> >  
> >  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > -	strscpy(v->card, tea->card, sizeof(v->card));
> > -	strlcat(v->card, tea->tea5759 ? " TEA5759" : " TEA5757", sizeof(v->card));
> > +
> > +	seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > +	seq_buf_puts(&s, (const char *)tea->card);
> > +	seq_buf_puts(&s, tea->tea5759 ? " TEA5759" : " TEA5757");
> > +	seq_buf_terminate(&s);
> > +
> >  	strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
> >  	return 0;
> >  }  
> 
> 

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Mariia Nikitash]

On Tue, Nov 10, 2026 at 5:54 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Tue, 10 Nov 2026 11:08:30 +0100
> Hans Verkuil <hverkuil+cisco@kernel.org> wrote:
>
> > On 06/11/2026 08:46, Mariia Nikitash wrote:
> > > In preparation for removing the deprecated strlcat() API[1],
> > > replace the strscpy() and strlcat() calls used to construct
> > > V4L2 card names in the TEA5777 and TEA575x radio drivers.
> > >
> > > seq_buf tracks the current write position and remaining capacity
> > > across appends, avoiding repeated scans of the destination buffer.
> > > This preserves the resulting strings, including truncation behavior.
> > >
> > > Link: https://github.com/KSPP/linux/issues/370 [1]
> > > Signed-off-by: Mariia Nikitash <nikitash.mariiaw@gmail.com>
> >
> > Nacked-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> >

Hi,



> > This is silly. Just use scnprintf().
>
> Or wait for someone to add a call that just does what is needed.
> In this case the first stscpy() returns the length (and it is often
> known).
> So you could have:
>         len = strscpy(buf, tea->card[, buflen]);
>         len = strscpy_at(buf, len, " TEA5777"[, buflen]);
> or:
>         len = strscat(buf, buflen, tea->card, " TEA5777", ....);
> or some variant that will do a strnlen(buf, buflen) to get the offset.
>
> But it really is best to leave the strlcat() calls in there for now.
>

Thanks Hans and David for the feedback.

I agree that seq_buf is too complex here. I'll look into using
scnprintf() as Hans suggested.

David, I also see your point about leaving strlcat() in place until a
more suitable append helper is available. I'll revisit the approach
before deciding on a v2.


Best,
Mariia

> David
>
> >
> > Regards,
> >
> >       Hans
> >
> > > ---
> > >  drivers/media/radio/radio-tea5777.c | 10 ++++++++--
> > >  drivers/media/radio/tea575x.c       | 10 ++++++++--
> > >  2 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/media/radio/radio-tea5777.c b/drivers/media/radio/radio-tea5777.c
> > > index fb9de7bbcd19..2b1c7f6ddd60 100644
> > > --- a/drivers/media/radio/radio-tea5777.c
> > > +++ b/drivers/media/radio/radio-tea5777.c
> > > @@ -13,6 +13,7 @@
> > >  #include <linux/init.h>
> > >  #include <linux/module.h>
> > >  #include <linux/sched.h>
> > > +#include <linux/seq_buf.h>
> > >  #include <linux/slab.h>
> > >  #include <media/v4l2-device.h>
> > >  #include <media/v4l2-dev.h>
> > > @@ -255,10 +256,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
> > >                                     struct v4l2_capability *v)
> > >  {
> > >     struct radio_tea5777 *tea = video_drvdata(file);
> > > +   struct seq_buf s;
> > >
> > >     strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > > -   strscpy(v->card, tea->card, sizeof(v->card));
> > > -   strlcat(v->card, " TEA5777", sizeof(v->card));
> > > +
> > > +   seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > > +   seq_buf_puts(&s, (const char *)tea->card);
> > > +   seq_buf_puts(&s, " TEA5777");
> > > +   seq_buf_terminate(&s);
> > > +
> > >     strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
> > >     return 0;
> > >  }
> > > diff --git a/drivers/media/radio/tea575x.c b/drivers/media/radio/tea575x.c
> > > index c37315226c42..e49228eb7c46 100644
> > > --- a/drivers/media/radio/tea575x.c
> > > +++ b/drivers/media/radio/tea575x.c
> > > @@ -9,6 +9,7 @@
> > >  #include <linux/module.h>
> > >  #include <linux/init.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/seq_buf.h>
> > >  #include <linux/sched.h>
> > >  #include <asm/io.h>
> > >  #include <media/v4l2-device.h>
> > > @@ -221,10 +222,15 @@ static int vidioc_querycap(struct file *file, void  *priv,
> > >                                     struct v4l2_capability *v)
> > >  {
> > >     struct snd_tea575x *tea = video_drvdata(file);
> > > +   struct seq_buf s;
> > >
> > >     strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > > -   strscpy(v->card, tea->card, sizeof(v->card));
> > > -   strlcat(v->card, tea->tea5759 ? " TEA5759" : " TEA5757", sizeof(v->card));
> > > +
> > > +   seq_buf_init(&s, (char *)v->card, sizeof(v->card));
> > > +   seq_buf_puts(&s, (const char *)tea->card);
> > > +   seq_buf_puts(&s, tea->tea5759 ? " TEA5759" : " TEA5757");
> > > +   seq_buf_terminate(&s);
> > > +
> > >     strscpy(v->bus_info, tea->bus_info, sizeof(v->bus_info));
> > >     return 0;
> > >  }
> >
> >
>

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[David Laight]

On Tue, 10 Nov 2026 10:41:06 -0800
Mariia Nikitash <nikitash.mariiaw@gmail.com> wrote:


> David, I also see your point about leaving strlcat() in place until a
> more suitable append helper is available. I'll revisit the approach
> before deciding on a v2.

I really don't understand why strlcat() got marked deprecated without
a suitable replacement.
The performance of any form of strcpy(t + strlen(t), s) is horrid.
The only extra issue strlcat() has is to do with it doing strlen(s)
to generated the return value that most code doesn't want
(and in pretty much all reasonable cases is actually fine).

David

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Kees Cook]

On Sun, Nov 08, 2026 at 10:25:35PM +0100, Alejandro Colomar wrote:
> Hi David,
> 
> > Date: 2026-11-08 21:02:32+0000
> > From: David Laight <david.laight.linux@gmail.com>
> >
> [...]
> > 
> > seq_buf is just OTT.
> > Using to replace a single strcat() is just silly.
> > There is no point doing these patches until there is a sensible alternative.
> 
> Agree.
> 
> [...]
> > >  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > > -	strscpy(v->card, tea->card, sizeof(v->card));
> > > -	strlcat(v->card, " TEA5777", sizeof(v->card));
> > 
> > What is wrong with (and almost OTT):
> > #define TEA5777 " TEA5777"
> > 	BUILD_BUG_ON(sizeof(TEA5777) <= sizeof(v->card))
> > 
> > 	len = strscpy(v->card, tea->card, sizeof(v->card) - sizeof(TEA5777) + 1);
> 
> I can answer this one: bugs (particularly, off by one) are quite prone
> in such code.
> 
> From the Authors of strlcat(3), you can look at OpenBSD's manual page:
> 
> 	Since it is known how many characters were copied the first
> 	time, things can be sped up a bit by using a copy instead of an
> 	append:
> 
> 		char *dir, *file, pname[PATH_MAX];
> 		size_t n;
> 
> 		...
> 
> 		n = strlcpy(pname, dir, sizeof(pname));
> 		if (n >= sizeof(pname))
> 			goto toolong;
> 		if (strlcpy(pname + n, file, sizeof(pname) - n) >= sizeof(pname) - n)
> 			goto toolong;
> 
> 	However, one may question the validity of such optimizations, as
> 	they defeat the whole purpose of strlcpy() and strlcat().  As a
> 	matter of fact, the first version of this manual page got it
> 	wrong.
> 
> > 	if (len < 0)
> > 		goto fail;
> > 	memcpy(v->card + len, TEA5777, sizeof(TEA5777);
> > 
> > What might be useful is a helper that would let you write:
> > 	len = strscpy(v->card, tea->card);
> > 	len = strscpy_at(v->card, len, " TEA5777" [, implied_length]);
> > 	if (len < 0)
> > 		goto fail;
> 
> Indeed, as said in other threads, I have such an API:
> 
> 	p = v->card;
> 	e = ARRAY_END(v->card);
> 	p = stpecpy(p, e, tea->card);
> 	p = stpecpy(p, e, " TEA5777");
> 	if (p == NULL)
> 		goto fail;
> 
> Linus didn't like my function name.  We could indeed call it strscpy_at().

I really just don't want to have bare pointers or lengths being used for
any of these manipulations. strlcat needs to go away because it's part
of the whole C string API that is confused about array vs pointer and
has no sense of handling bounds. Yes, FORTIFY does its best, but between
lack of sane bounds and doing a strlen for every action, it has to go.

And I'd like _fewer_ APIs (I realize that didn't happen at all for
seq_buf, but that our current best direction). And I especially don't
want a "len" getting passed around. It needs bounds to be *in* the data
type. And that means either true arrays, or internal bounds tracking
(like seq_buf), and it shouldn't have "extra work" associated with it
(e.g. stpecpy above still has to find the current end of the C string).
C strings were a mistake, but here we are.

So, for "easy" constructions, sprintf-family, and for complex
constructions, seq_buf. And probably the large bit of thought at each
place where it is possible, is "how should truncation be communicated?"

-Kees

-- 
Kees Cook

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[David Laight]

On Thu, 12 Nov 2026 09:54:15 -0800
Kees Cook <kees@kernel.org> wrote:

> On Sun, Nov 08, 2026 at 10:25:35PM +0100, Alejandro Colomar wrote:
> > Hi David,
> >   
> > > Date: 2026-11-08 21:02:32+0000
> > > From: David Laight <david.laight.linux@gmail.com>
> > >  
> > [...]  
> > > 
> > > seq_buf is just OTT.
> > > Using to replace a single strcat() is just silly.
> > > There is no point doing these patches until there is a sensible alternative.  
> > 
> > Agree.
> > 
> > [...]  
> > > >  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > > > -	strscpy(v->card, tea->card, sizeof(v->card));
> > > > -	strlcat(v->card, " TEA5777", sizeof(v->card));  
> > > 
> > > What is wrong with (and almost OTT):
> > > #define TEA5777 " TEA5777"
> > > 	BUILD_BUG_ON(sizeof(TEA5777) <= sizeof(v->card))
> > > 
> > > 	len = strscpy(v->card, tea->card, sizeof(v->card) - sizeof(TEA5777) + 1);  
> > 
> > I can answer this one: bugs (particularly, off by one) are quite prone
> > in such code.
> > 
> > From the Authors of strlcat(3), you can look at OpenBSD's manual page:
> > 
> > 	Since it is known how many characters were copied the first
> > 	time, things can be sped up a bit by using a copy instead of an
> > 	append:
> > 
> > 		char *dir, *file, pname[PATH_MAX];
> > 		size_t n;
> > 
> > 		...
> > 
> > 		n = strlcpy(pname, dir, sizeof(pname));
> > 		if (n >= sizeof(pname))
> > 			goto toolong;
> > 		if (strlcpy(pname + n, file, sizeof(pname) - n) >= sizeof(pname) - n)
> > 			goto toolong;
> > 
> > 	However, one may question the validity of such optimizations, as
> > 	they defeat the whole purpose of strlcpy() and strlcat().  As a
> > 	matter of fact, the first version of this manual page got it
> > 	wrong.
> >   
> > > 	if (len < 0)
> > > 		goto fail;
> > > 	memcpy(v->card + len, TEA5777, sizeof(TEA5777);
> > > 
> > > What might be useful is a helper that would let you write:
> > > 	len = strscpy(v->card, tea->card);
> > > 	len = strscpy_at(v->card, len, " TEA5777" [, implied_length]);
> > > 	if (len < 0)
> > > 		goto fail;  
> > 
> > Indeed, as said in other threads, I have such an API:
> > 
> > 	p = v->card;
> > 	e = ARRAY_END(v->card);
> > 	p = stpecpy(p, e, tea->card);
> > 	p = stpecpy(p, e, " TEA5777");
> > 	if (p == NULL)
> > 		goto fail;

That one is playing with pointers.
They do make it harder to do any kind of compile-time (or compler helped)
sanity checks.

> > Linus didn't like my function name.  We could indeed call it strscpy_at().  
> 
> I really just don't want to have bare pointers or lengths being used for
> any of these manipulations. strlcat needs to go away because it's part
> of the whole C string API that is confused about array vs pointer and
> has no sense of handling bounds. Yes, FORTIFY does its best, but between
> lack of sane bounds and doing a strlen for every action, it has to go.

But both snprintf() and seq_buf are far more expensive than doing the
strlen().
There is the separate issue that strlcpy() manages to needlessly inline a
strnlen() in a lot of the call sites - instead of deferring it to the
helper function.

David

> 
> And I'd like _fewer_ APIs (I realize that didn't happen at all for
> seq_buf, but that our current best direction). And I especially don't
> want a "len" getting passed around. It needs bounds to be *in* the data
> type. And that means either true arrays, or internal bounds tracking
> (like seq_buf), and it shouldn't have "extra work" associated with it
> (e.g. stpecpy above still has to find the current end of the C string).
> C strings were a mistake, but here we are.
> 
> So, for "easy" constructions, sprintf-family, and for complex
> constructions, seq_buf. And probably the large bit of thought at each
> place where it is possible, is "how should truncation be communicated?"
> 
> -Kees
> 

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Kees Cook]

On Thu, Nov 12, 2026 at 07:07:02PM +0000, David Laight wrote:
> But both snprintf() and seq_buf are far more expensive than doing the
> strlen().
> There is the separate issue that strlcpy() manages to needlessly inline a
> strnlen() in a lot of the call sites - instead of deferring it to the
> helper function.

Right, my driving goal here isn't about performance, it's about
correctness (as much as possible given our giant codebase). We don't
need the most perfect fast implementation if it's too easy to shoot our
feet. strlcat() requires an external bounds that has no strict tie to
the destination argument, and it has no bounds information at all for
the source argument.

If we're in fast-path, we have a different set of criteria. And seq_buf
was actually designed for that, as it trades storage for speed: it keeps
the needed offsets in memory, but that makes it fast for stuff like
perf.

-- 
Kees Cook

Re: [PATCH] media: radio: Replace strlcat() with seq_buf in TEA drivers

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 8756 bytes --]

Hi Kees,

> Date: 2026-11-12 09:54:15-0800
> From: Kees Cook <kees@kernel.org>
>
> On Sun, Nov 08, 2026 at 10:25:35PM +0100, Alejandro Colomar wrote:
> > Hi David,
> > 
> > > Date: 2026-11-08 21:02:32+0000
> > > From: David Laight <david.laight.linux@gmail.com>
> > >
[...]
> > > >  	strscpy(v->driver, tea->v4l2_dev->name, sizeof(v->driver));
> > > > -	strscpy(v->card, tea->card, sizeof(v->card));
> > > > -	strlcat(v->card, " TEA5777", sizeof(v->card));
> > > 
> > > What is wrong with (and almost OTT):
> > > #define TEA5777 " TEA5777"
> > > 	BUILD_BUG_ON(sizeof(TEA5777) <= sizeof(v->card))
> > > 
> > > 	len = strscpy(v->card, tea->card, sizeof(v->card) - sizeof(TEA5777) + 1);
> > 
> > I can answer this one: bugs (particularly, off by one) are quite prone
> > in such code.
> > 
> > From the Authors of strlcat(3), you can look at OpenBSD's manual page:
> > 
> > 	Since it is known how many characters were copied the first
> > 	time, things can be sped up a bit by using a copy instead of an
> > 	append:
> > 
> > 		char *dir, *file, pname[PATH_MAX];
> > 		size_t n;
> > 
> > 		...
> > 
> > 		n = strlcpy(pname, dir, sizeof(pname));
> > 		if (n >= sizeof(pname))
> > 			goto toolong;
> > 		if (strlcpy(pname + n, file, sizeof(pname) - n) >= sizeof(pname) - n)
> > 			goto toolong;
> > 
> > 	However, one may question the validity of such optimizations, as
> > 	they defeat the whole purpose of strlcpy() and strlcat().  As a
> > 	matter of fact, the first version of this manual page got it
> > 	wrong.
> > 
> > > 	if (len < 0)
> > > 		goto fail;
> > > 	memcpy(v->card + len, TEA5777, sizeof(TEA5777);
> > > 
> > > What might be useful is a helper that would let you write:
> > > 	len = strscpy(v->card, tea->card);
> > > 	len = strscpy_at(v->card, len, " TEA5777" [, implied_length]);
> > > 	if (len < 0)
> > > 		goto fail;
> > 
> > Indeed, as said in other threads, I have such an API:
> > 
> > 	p = v->card;
> > 	e = ARRAY_END(v->card);
> > 	p = stpecpy(p, e, tea->card);
> > 	p = stpecpy(p, e, " TEA5777");
> > 	if (p == NULL)
> > 		goto fail;
> > 
> > Linus didn't like my function name.  We could indeed call it strscpy_at().
> 
> I really just don't want to have bare pointers or lengths being used for
> any of these manipulations.


> strlcat needs to go away because it's part
> of the whole C string API that is confused about array vs pointer and
> has no sense of handling bounds. Yes, FORTIFY does its best, but between
> lack of sane bounds and doing a strlen for every action, it has to go.

I disagree that the whole C string API is confused.  In the future,
when array parameters become actual arrays (so you can use _Countof() on
them), you'll have better language support for that.

strlcat(3) indeed has some problems, and has to go, but I would like to
not put the whole C string API in that sack.

> 
> And I'd like _fewer_ APIs

That's a mistake, IMO.  The reason why programmers write bugs using the
string APIs is precisely because they try to use few APIs, and try to
use them even in places where a more appropriate API would be better.

Take for example strncpy(3).  It has been abused in so many ways, and we
all know the results of that.  Now look at what you've done in the
kernel: it's been replaced by around a dozen different APIs.  Quoting
the documentation:

	The replacements for strncpy() are:

	- strscpy() when the destination must be NUL-terminated.
	- strscpy_pad() when the destination must be NUL-terminated and
	  zero-padded (e.g., structs crossing privilege boundaries).
	- memtostr() for NUL-terminated destinations from non-NUL-terminated
	  fixed-width sources (with the `__nonstring` attribute on the source).
	- memtostr_pad() for the same, but with zero-padding.
	- strtomem() for non-NUL-terminated fixed-width destinations, with
	  the `__nonstring` attribute on the destination.
	- strtomem_pad() for non-NUL-terminated destinations that also need
	  zero-padding.
	- memcpy_and_pad() for bounded copies from potentially unterminated
	  sources where the destination size is a runtime value.

And that is the right thing to do.  In parallel, we've developed
fundamentally the same functions in shadow-utils.  I think that's where
research converges to: one API for each different use case.  That makes
the APIs more descriptive of the action they perform, which let's the
programmer better understand the code.  They're also way more robust, as
you can add many safety checks in the macros that implement them.

> (I realize that didn't happen at all for
> seq_buf, but that our current best direction).

I think the problem in seq_buf is not the amount of functions, but
rather the fact that it tries to support both strings and nonstrings at
the same time.  That's literally repeating the mistake of strncpy(3)
with this new type.  You may try to patch it with tape, but it's
fundamentally flawed, and it will eventually show up (we've already seen
the bugs in its implementation).

> And I especially don't
> want a "len" getting passed around.

I partially agree with this.  I don't like passing a length, because
lengths and sizes are too problematic.  However, I would like you to
trust me that pointers aren't that bad.  We've been using them in
shadow-utils extensively, and they're super-comfortable if the functions
are well designed.

> It needs bounds to be *in* the data
> type. And that means either true arrays,

Indeed, there's work towards that.  I'll show some preview of what may
come in future dialects of C.

Now, we can have a stpecpy() function:

	char *
	stpecpy(char dst[], const char end[0], const char *restrict src)
	{
		ssize_t  dlen;

		if (dst == NULL)
			return NULL;
		dlen = strscpy(dst, src, end - dst);
		if (dlen < 0)
			return NULL;

		return dst + dlen;
	}

In GCC, you can improve the prototype:

	char *
	stpecpy(char dst[], const char end[];
	    char dst[dst?end-dst:0], const char end[0], const char *restrict src)

With a patch I'm working on for GCC (it might land in GCC 17 if we're
lucky, but it's more likely to end up in GCC 18), we'll be able to use
_Countof() on array parameters, so we could use the two-arg version of
strscpy() for implementing this:

	char *
	stpecpy(char dst[], const char end[];
	    char dst[dst?end-dst:0], const char end[0], const char *restrict src)
	{
		ssize_t  dlen;

		if (dst == NULL)
			return NULL;
		dlen = strscpy(dst, src);
		if (dlen < 0)
			return NULL;

		return dst + dlen;
	}

And in a few years, we might end up having slices in C, which would
allow having the same functionality with just 2 arguments:

	char
	stpecpy(char dst[:], const char *restrict src)[:]
	{
		ssize_t  dlen;

		if (dst == NULL)
			return NULL;
		dlen = strscpy(dst, src);
		if (dlen == -1)
			return NULL;

		return dst + dlen;
	}

So, there's a bright future for traditional C string APIs.  And it's not
too far away.

> or internal bounds tracking
> (like seq_buf),

I believe that's going to bite eventually, and it will also be difficult
to return to the simplicity of the C string API once we have these few
features that make it safe.

> and it shouldn't have "extra work" associated with it
> (e.g. stpecpy above still has to find the current end of the C string).

No!  stpecpy() is really the same as strscpy(9).  It's *not* a cat
function.  See the implementation(s) above: there's no strlen(3) in it.

The closest relative to it is stpcpy(3) from POSIX, which is really just
strcpy(3), but returning a pointer that encodes the position.  That was
designed precisely to solve the performance issues of strcat(3).

> C strings were a mistake, but here we are.

Trust me: it's not.  The problems with C strings are, as you said above,
the limited support for array parameters in the language.  But it's not
too difficult to solve that problem.  And once that is solved, magically
the traditional string APIs become much better than the structure types,
by being at least as safe, but much simpler.

So, I'd like you to trust me to add stpecpy() --whatever we end up
calling it--, and perhaps the sprintf() variant of it, and see how it
really improves the code.  In the coming years, when we solve the issues
with array parameters in the language, they will become even better.

> So, for "easy" constructions, sprintf-family, and for complex
> constructions, seq_buf. And probably the large bit of thought at each
> place where it is possible, is "how should truncation be communicated?"
> 
> -Kees


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]