[RFC v1] seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

[RFC v1] seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

[Alejandro Colomar]

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

This implements DECLARE_SEQ_BUF() in a way that is independent of the
members of the structure.

This will allow transitioning to a different implementation of seq_buf
which uses pointers instead of sizes, on which I plan to work soon.

The compiler should be able to optimize this out and produce the same
code as before.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

Hi Steve, Kees,

This is a patch that I could split from the rewrite using pointers, so
we can apply it earlier to reduce the later diff.

I've built a kernel with 'allmodconfig' to check that it's okay.

The reason I need this patch is that we can't calculate the end pointer
of the compound literal without passing it to a function.  If you want
me to hold this patch and send it together with the rest of the rewrite,
I can also do that, but I thought this might be useful by itself.


Have a lovely night!
Alex


 include/linux/seq_buf.h | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 9f2839e73f8a..facc5643c049 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -25,10 +25,7 @@ struct seq_buf {
 };
 
 #define DECLARE_SEQ_BUF(NAME, SIZE)			\
-	struct seq_buf NAME = {				\
-		.buffer = (char[SIZE]) { 0 },		\
-		.size = SIZE,				\
-	}
+	struct seq_buf NAME = seq_buf_create((char[SIZE]){0}, SIZE)
 
 static inline void seq_buf_clear(struct seq_buf *s)
 {
@@ -45,6 +42,15 @@ seq_buf_init(struct seq_buf *s, char *buf, unsigned int size)
 	seq_buf_clear(s);
 }
 
+static inline struct seq_buf
+seq_buf_create(char *buf, unsigned int size)
+{
+	struct seq_buf  s;
+
+	seq_buf_init(&s, buf, size);
+	return s;
+}
+
 /*
  * seq_buf have a buffer that might overflow. When this happens
  * len is set to be greater than size.

Range-diff against v0:
-:  ------------ > 1:  9bda534b33f0 seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()
-- 
2.55.0


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

Re: [RFC v1] seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

[Steven Rostedt]

On Thu, 29 Oct 2026 00:19:24 +0100
Alejandro Colomar <alx@kernel.org> wrote:

> This implements DECLARE_SEQ_BUF() in a way that is independent of the
> members of the structure.
> 
> This will allow transitioning to a different implementation of seq_buf
> which uses pointers instead of sizes, on which I plan to work soon.
> 
> The compiler should be able to optimize this out and produce the same
> code as before.
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
> 
> Hi Steve, Kees,
> 
> This is a patch that I could split from the rewrite using pointers, so
> we can apply it earlier to reduce the later diff.
> 
> I've built a kernel with 'allmodconfig' to check that it's okay.
> 
> The reason I need this patch is that we can't calculate the end pointer
> of the compound literal without passing it to a function.  If you want
> me to hold this patch and send it together with the rest of the rewrite,
> I can also do that, but I thought this might be useful by itself.
> 
> 
> Have a lovely night!
> Alex
> 
> 
>  include/linux/seq_buf.h | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
> index 9f2839e73f8a..facc5643c049 100644
> --- a/include/linux/seq_buf.h
> +++ b/include/linux/seq_buf.h
> @@ -25,10 +25,7 @@ struct seq_buf {
>  };
>  
>  #define DECLARE_SEQ_BUF(NAME, SIZE)			\
> -	struct seq_buf NAME = {				\
> -		.buffer = (char[SIZE]) { 0 },		\
> -		.size = SIZE,				\
> -	}
> +	struct seq_buf NAME = seq_buf_create((char[SIZE]){0}, SIZE)

It can't use the DECLARE_* name as that is by default something that
can be used outside of a function:

  DECLARE_SEQ_BUF(foo, 1024);

Needs to work. Just because nothing does that now, doesn't mean it will
not happen in the future. From my understanding, all DECLARE_* macros
for other types work this way.

-- Steve


>  
>  static inline void seq_buf_clear(struct seq_buf *s)
>  {
> @@ -45,6 +42,15 @@ seq_buf_init(struct seq_buf *s, char *buf, unsigned int size)
>  	seq_buf_clear(s);
>  }
>  
> +static inline struct seq_buf
> +seq_buf_create(char *buf, unsigned int size)
> +{
> +	struct seq_buf  s;
> +
> +	seq_buf_init(&s, buf, size);
> +	return s;
> +}
> +
>  /*
>   * seq_buf have a buffer that might overflow. When this happens
>   * len is set to be greater than size.
> 
> Range-diff against v0:
> -:  ------------ > 1:  9bda534b33f0 seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

Re: [RFC v1] seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

[Alejandro Colomar]

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

Hi Steve,

> Date: 2026-10-28 21:12:27-0400
> From: Steven Rostedt <rostedt@goodmis.org>
>
> On Thu, 29 Oct 2026 00:19:24 +0100
> Alejandro Colomar <alx@kernel.org> wrote:
> 
> > This implements DECLARE_SEQ_BUF() in a way that is independent of the
> > members of the structure.
> > 
> > This will allow transitioning to a different implementation of seq_buf
> > which uses pointers instead of sizes, on which I plan to work soon.
> > 
> > The compiler should be able to optimize this out and produce the same
> > code as before.
> > 
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Nathan Lynch <nathanl@linux.ibm.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Alejandro Colomar <alx@kernel.org>
> > ---
> > 
> > Hi Steve, Kees,
> > 
> > This is a patch that I could split from the rewrite using pointers, so
> > we can apply it earlier to reduce the later diff.
> > 
> > I've built a kernel with 'allmodconfig' to check that it's okay.
> > 
> > The reason I need this patch is that we can't calculate the end pointer
> > of the compound literal without passing it to a function.  If you want
> > me to hold this patch and send it together with the rest of the rewrite,
> > I can also do that, but I thought this might be useful by itself.
> > 
> > 
> > Have a lovely night!
> > Alex
> > 
> > 
> >  include/linux/seq_buf.h | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
> > index 9f2839e73f8a..facc5643c049 100644
> > --- a/include/linux/seq_buf.h
> > +++ b/include/linux/seq_buf.h
> > @@ -25,10 +25,7 @@ struct seq_buf {
> >  };
> >  
> >  #define DECLARE_SEQ_BUF(NAME, SIZE)			\
> > -	struct seq_buf NAME = {				\
> > -		.buffer = (char[SIZE]) { 0 },		\
> > -		.size = SIZE,				\
> > -	}
> > +	struct seq_buf NAME = seq_buf_create((char[SIZE]){0}, SIZE)
> 
> It can't use the DECLARE_* name as that is by default something that
> can be used outside of a function:
> 
>   DECLARE_SEQ_BUF(foo, 1024);
> 
> Needs to work. Just because nothing does that now, doesn't mean it will
> not happen in the future. From my understanding, all DECLARE_* macros
> for other types work this way.

Hmmmm, then I would like to remove this macro entirely.  It is not
possible to implement it for a structure that has '.p' and '.end'
without a function call.

It would only be implementable with constexpr (or constinit? C++ has too
many constsomething things that slightly differ, and I don't know which
is which) functions that can be called at file scope, but those don't
exist in C.

Would it be okay for you to remove this, and perhaps replace it with
direct calls to seq_buf_create()?  Maybe like this:

	#define SEQ_BUF_CREATE(SIZE)  seq_buf_create((char[SIZE]){0}, SIZE)

	...

	int
	main(void)
	{
		struct seq_buf foo = SEQ_BUF_CREATE(1024);
	}


Have a lovely day!
Alex

> 
> -- Steve
> 
> 
> >  
> >  static inline void seq_buf_clear(struct seq_buf *s)
> >  {
> > @@ -45,6 +42,15 @@ seq_buf_init(struct seq_buf *s, char *buf, unsigned int size)
> >  	seq_buf_clear(s);
> >  }
> >  
> > +static inline struct seq_buf
> > +seq_buf_create(char *buf, unsigned int size)
> > +{
> > +	struct seq_buf  s;
> > +
> > +	seq_buf_init(&s, buf, size);
> > +	return s;
> > +}
> > +
> >  /*
> >   * seq_buf have a buffer that might overflow. When this happens
> >   * len is set to be greater than size.
> > 
> > Range-diff against v0:
> > -:  ------------ > 1:  9bda534b33f0 seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()
> 
> 

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

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

Re: [RFC v1] seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

[Steven Rostedt]

On Thu, 29 Oct 2026 12:33:19 +0100
Alejandro Colomar <alx+linux-hardening@kernel.org> wrote:

> > > diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
> > > index 9f2839e73f8a..facc5643c049 100644
> > > --- a/include/linux/seq_buf.h
> > > +++ b/include/linux/seq_buf.h
> > > @@ -25,10 +25,7 @@ struct seq_buf {
> > >  };
> > >  
> > >  #define DECLARE_SEQ_BUF(NAME, SIZE)			\
> > > -	struct seq_buf NAME = {				\
> > > -		.buffer = (char[SIZE]) { 0 },		\
> > > -		.size = SIZE,				\
> > > -	}
> > > +	struct seq_buf NAME = seq_buf_create((char[SIZE]){0}, SIZE)  
> > 
> > It can't use the DECLARE_* name as that is by default something that
> > can be used outside of a function:
> > 
> >   DECLARE_SEQ_BUF(foo, 1024);
> > 
> > Needs to work. Just because nothing does that now, doesn't mean it will
> > not happen in the future. From my understanding, all DECLARE_* macros
> > for other types work this way.  
> 
> Hmmmm, then I would like to remove this macro entirely.  It is not
> possible to implement it for a structure that has '.p' and '.end'
> without a function call.
> 
> It would only be implementable with constexpr (or constinit? C++ has too
> many constsomething things that slightly differ, and I don't know which
> is which) functions that can be called at file scope, but those don't
> exist in C.
> 
> Would it be okay for you to remove this, and perhaps replace it with
> direct calls to seq_buf_create()?  Maybe like this:
> 
> 	#define SEQ_BUF_CREATE(SIZE)  seq_buf_create((char[SIZE]){0}, SIZE)

No.

Honestly, you haven't convinced me that the pointer logic is any better. It
looks to me that it may be just be moving the complexity elsewhere.

Yes, using indexes can be error prone, but if you keep all that abstracted
out to make sure you can validate that it works, it should be fine. Nothing
that uses seq_buf should care about the internals. Using indexes should not
be a point of failure as it's not extended outside the scope of the
internals of seq_buf.

That is, once you prove the internals of seq_buf is correct, then you are done.

-- Steve


> 
> 	...
> 
> 	int
> 	main(void)
> 	{
> 		struct seq_buf foo = SEQ_BUF_CREATE(1024);
> 	}

Re: [RFC v1] seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

[Alejandro Colomar]

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

Hi Steve,

> Date: 2026-10-29 09:41:56-0400
> From: Steven Rostedt <rostedt@goodmis.org>
>
> On Thu, 29 Oct 2026 12:33:19 +0100
> Alejandro Colomar <alx+linux-hardening@kernel.org> wrote:
> 
> > > > diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
> > > > index 9f2839e73f8a..facc5643c049 100644
> > > > --- a/include/linux/seq_buf.h
> > > > +++ b/include/linux/seq_buf.h
> > > > @@ -25,10 +25,7 @@ struct seq_buf {
> > > >  };
> > > >  
> > > >  #define DECLARE_SEQ_BUF(NAME, SIZE)			\
> > > > -	struct seq_buf NAME = {				\
> > > > -		.buffer = (char[SIZE]) { 0 },		\
> > > > -		.size = SIZE,				\
> > > > -	}
> > > > +	struct seq_buf NAME = seq_buf_create((char[SIZE]){0}, SIZE)  
> > > 
> > > It can't use the DECLARE_* name as that is by default something that
> > > can be used outside of a function:
> > > 
> > >   DECLARE_SEQ_BUF(foo, 1024);
> > > 
> > > Needs to work. Just because nothing does that now, doesn't mean it will
> > > not happen in the future. From my understanding, all DECLARE_* macros
> > > for other types work this way.  
> > 
> > Hmmmm, then I would like to remove this macro entirely.  It is not
> > possible to implement it for a structure that has '.p' and '.end'
> > without a function call.
> > 
> > It would only be implementable with constexpr (or constinit? C++ has too
> > many constsomething things that slightly differ, and I don't know which
> > is which) functions that can be called at file scope, but those don't
> > exist in C.
> > 
> > Would it be okay for you to remove this, and perhaps replace it with
> > direct calls to seq_buf_create()?  Maybe like this:
> > 
> > 	#define SEQ_BUF_CREATE(SIZE)  seq_buf_create((char[SIZE]){0}, SIZE)
> 
> No.
> 
> Honestly, you haven't convinced me that the pointer logic is any better. It
> looks to me that it may be just be moving the complexity elsewhere.

The complexity doesn't move elsewhere, IMO.  It has an issue with file-
scope initialization, indeed, because derived pointers cannot be
constant expressions, and thus this precise problem, but other than
that, there's no off-by-one-prone edges in that design.

However, I understand the fear, so I'll try to prove the simplicity
before asking you to trust me about it.  (I guess by doing a full
conversion, and showing that the diff reduces code without showing
corner cases like the bug we're discussing at the moment in the other
thread.)

> Yes, using indexes can be error prone, but if you keep all that abstracted
> out to make sure you can validate that it works, it should be fine. Nothing
> that uses seq_buf should care about the internals.

Actually, there are a few places where those internals somewhat already
escape the API.  For example:

	avail = seq_buf_get_buf(s, &buf);
	seq_buf_init(&ns, buf, avail);

could be rewritten to use a single function (we can already do that):

	seq_buf_get_tail(&ns, s);

This way, we don't need to have a temporary 'char *buf' that we don't
really need in the caller, and we also avoid inspecting the available
size.

This could be done already with this:

	diff --git i/include/linux/seq_buf.h w/include/linux/seq_buf.h
	index f006f2654e4c..f7ee062374bf 100644
	--- i/include/linux/seq_buf.h
	+++ w/include/linux/seq_buf.h
	@@ -58,6 +58,16 @@ seq_buf_init(struct seq_buf *s, char *buf, unsigned int size)
		seq_buf_clear(s);
	 }
	 
	+static inline void
	+seq_buf_get_tail(struct seq_buf *tail, const struct seq_buf *s)
	+{
	+       char *p;
	+       size_t size;
	+
	+       size = seq_buf_get_buf(s, &p);
	+       seq_buf_init(tail, p, size);
	+}
	+
	 /*
	  * seq_buf have a buffer that might overflow. When this happens
	  * len is set to be greater than size.

> Using indexes should not
> be a point of failure as it's not extended outside the scope of the
> internals of seq_buf.

Except that many places do look at the size by looking at the return
value of things like seq_buf_get_buf().

> That is, once you prove the internals of seq_buf is correct, then you are done.

And also except that you still need to prove the internals of seq_buf,
which aren't exactly trivial.  With pointers, those internals become
simpler.

I have a draft for moving to pointers, and it cleans the implementation
quite a lot:

	 include/linux/seq_buf.h |  86 +++++++++++----------------
	 lib/seq_buf.c           | 101 ++++++++++----------------------
	 2 files changed, 66 insertions(+), 121 deletions(-)

(It's far from ready.)


Cheers,
Alex

> 
> -- Steve
> 
> 
> > 
> > 	...
> > 
> > 	int
> > 	main(void)
> > 	{
> > 		struct seq_buf foo = SEQ_BUF_CREATE(1024);
> > 	}
> 

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

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

Re: [RFC v1] seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

[Steven Rostedt]

On Thu, 29 Oct 2026 15:15:29 +0100
Alejandro Colomar <alx+linux-hardening@kernel.org> wrote:

> > Honestly, you haven't convinced me that the pointer logic is any better. It
> > looks to me that it may be just be moving the complexity elsewhere.  
> 
> The complexity doesn't move elsewhere, IMO.  It has an issue with file-
> scope initialization, indeed, because derived pointers cannot be
> constant expressions, and thus this precise problem, but other than
> that, there's no off-by-one-prone edges in that design.

If you have to change the current API to satisfy the constraints of a new
implementation, that is moving the complexity.

> 
> However, I understand the fear, so I'll try to prove the simplicity
> before asking you to trust me about it.  (I guess by doing a full
> conversion, and showing that the diff reduces code without showing
> corner cases like the bug we're discussing at the moment in the other
> thread.)
> 
> > Yes, using indexes can be error prone, but if you keep all that abstracted
> > out to make sure you can validate that it works, it should be fine. Nothing
> > that uses seq_buf should care about the internals.  
> 
> Actually, there are a few places where those internals somewhat already
> escape the API.  For example:
> 
> 	avail = seq_buf_get_buf(s, &buf);
> 	seq_buf_init(&ns, buf, avail);

How is this exposing the internals? It's an API and it only modifies what
the user has control of (the buffer supplied in the first place).

> 
> could be rewritten to use a single function (we can already do that):
> 
> 	seq_buf_get_tail(&ns, s);
> 
> This way, we don't need to have a temporary 'char *buf' that we don't
> really need in the caller, and we also avoid inspecting the available
> size.

What happens if there's no size left?

> 
> This could be done already with this:
> 
> 	diff --git i/include/linux/seq_buf.h w/include/linux/seq_buf.h
> 	index f006f2654e4c..f7ee062374bf 100644
> 	--- i/include/linux/seq_buf.h
> 	+++ w/include/linux/seq_buf.h
> 	@@ -58,6 +58,16 @@ seq_buf_init(struct seq_buf *s, char *buf, unsigned int size)
> 		seq_buf_clear(s);
> 	 }
> 	 
> 	+static inline void
> 	+seq_buf_get_tail(struct seq_buf *tail, const struct seq_buf *s)
> 	+{
> 	+       char *p;
> 	+       size_t size;
> 	+
> 	+       size = seq_buf_get_buf(s, &p);
> 	+       seq_buf_init(tail, p, size);
> 	+}
> 	+
> 	 /*
> 	  * seq_buf have a buffer that might overflow. When this happens
> 	  * len is set to be greater than size.
> 
> > Using indexes should not
> > be a point of failure as it's not extended outside the scope of the
> > internals of seq_buf.  
> 
> Except that many places do look at the size by looking at the return
> value of things like seq_buf_get_buf().

That's not the internals. And your pointer algorithm had better supply the
same API. The amount of buffer left has nothing to do with the
implementation of seq_buf.

> 
> > That is, once you prove the internals of seq_buf is correct, then you are done.  
> 
> And also except that you still need to prove the internals of seq_buf,
> which aren't exactly trivial.  With pointers, those internals become
> simpler.
> 
> I have a draft for moving to pointers, and it cleans the implementation
> quite a lot:
> 
> 	 include/linux/seq_buf.h |  86 +++++++++++----------------
> 	 lib/seq_buf.c           | 101 ++++++++++----------------------
> 	 2 files changed, 66 insertions(+), 121 deletions(-)
> 
> (It's far from ready.)

Don't bother. I don't think you understand the usage of seq_buf. I also
already don't like the changes and I don't see why they are even necessary.

-- Steve

Re: [RFC v1] seq_buf: Add seq_buf_create() to implement DECLARE_SEQ_BUF()

[Alejandro Colomar]

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

Hi Steve,

> Date: 2026-10-29 10:28:30-0400
> From: Steven Rostedt <rostedt@goodmis.org>
>
> On Thu, 29 Oct 2026 15:15:29 +0100
> Alejandro Colomar <alx+linux-hardening@kernel.org> wrote:
> 
> > > Honestly, you haven't convinced me that the pointer logic is any better. It
> > > looks to me that it may be just be moving the complexity elsewhere.  
> > 
> > The complexity doesn't move elsewhere, IMO.  It has an issue with file-
> > scope initialization, indeed, because derived pointers cannot be
> > constant expressions, and thus this precise problem, but other than
> > that, there's no off-by-one-prone edges in that design.
> 
> If you have to change the current API to satisfy the constraints of a new
> implementation, that is moving the complexity.

It adds some small churn, but I wouldn't call that "complexity", given
that it actually reduces the complexity of the callers too.

> > However, I understand the fear, so I'll try to prove the simplicity
> > before asking you to trust me about it.  (I guess by doing a full
> > conversion, and showing that the diff reduces code without showing
> > corner cases like the bug we're discussing at the moment in the other
> > thread.)
> > 
> > > Yes, using indexes can be error prone, but if you keep all that abstracted
> > > out to make sure you can validate that it works, it should be fine. Nothing
> > > that uses seq_buf should care about the internals.  
> > 
> > Actually, there are a few places where those internals somewhat already
> > escape the API.  For example:
> > 
> > 	avail = seq_buf_get_buf(s, &buf);
> > 	seq_buf_init(&ns, buf, avail);
> 
> How is this exposing the internals? It's an API and it only modifies what
> the user has control of (the buffer supplied in the first place).

It would be better to not 

> > could be rewritten to use a single function (we can already do that):
> > 
> > 	seq_buf_get_tail(&ns, s);
> > 
> > This way, we don't need to have a temporary 'char *buf' that we don't
> > really need in the caller, and we also avoid inspecting the available
> > size.
> 
> What happens if there's no size left?

More or less the same as now, I think.

Since I don't have a tree with the recent patches from Kees, I don't
know if that has changed recently, but from what's in v7.3, I think
it will behave as this:

-  If the buffer has already overflowed (len>size+1), it will WARN().
   Then, it will produce a null pointer, and a size of 0.

-  If the buffer is exactly full, then it will also produce a null
   pointer and a size of 0, but without WARN().

With pointers, it could do something like this:

	static inline void
	seq_buf_get_tail(struct seq_buf *tail, const struct seq_buf *s)
	{
		if (seq_buf_buffer_left(s)) {
			tail->buffer = s->p;
			tail->p = s->p;
			tail->end = s->end;
		} else {
			WARN_ON(seq_buf_has_overflowed(s));
			tail->buffer = (static char[1]){""};
			tail->p = NULL;
			tail->p = tail->buffer + 1;
		}
	}

Which gives you an overflowed buffer, which is safe to hold.  One can
read it, and will read an empty string.  Any attempts to write to it
would be no-ops, since it's already overflowed.

> > This could be done already with this:
> > 
> > 	diff --git i/include/linux/seq_buf.h w/include/linux/seq_buf.h
> > 	index f006f2654e4c..f7ee062374bf 100644
> > 	--- i/include/linux/seq_buf.h
> > 	+++ w/include/linux/seq_buf.h
> > 	@@ -58,6 +58,16 @@ seq_buf_init(struct seq_buf *s, char *buf, unsigned int size)
> > 		seq_buf_clear(s);
> > 	 }
> > 	 
> > 	+static inline void
> > 	+seq_buf_get_tail(struct seq_buf *tail, const struct seq_buf *s)
> > 	+{
> > 	+       char *p;
> > 	+       size_t size;
> > 	+
> > 	+       size = seq_buf_get_buf(s, &p);
> > 	+       seq_buf_init(tail, p, size);
> > 	+}
> > 	+
> > 	 /*
> > 	  * seq_buf have a buffer that might overflow. When this happens
> > 	  * len is set to be greater than size.
> > 
> > > Using indexes should not
> > > be a point of failure as it's not extended outside the scope of the
> > > internals of seq_buf.  
> > 
> > Except that many places do look at the size by looking at the return
> > value of things like seq_buf_get_buf().
> 
> That's not the internals. And your pointer algorithm had better supply the
> same API.

The pointer algorithm presents the remaining size with
seq_buf_buffer_left(), but doesn't expose it when getting the tail of
a buffer, because I didn't find it necessary.  We could expose it there
too, though.

> The amount of buffer left has nothing to do with the
> implementation of seq_buf.
> 
> > 
> > > That is, once you prove the internals of seq_buf is correct, then you are done.  
> > 
> > And also except that you still need to prove the internals of seq_buf,
> > which aren't exactly trivial.  With pointers, those internals become
> > simpler.
> > 
> > I have a draft for moving to pointers, and it cleans the implementation
> > quite a lot:
> > 
> > 	 include/linux/seq_buf.h |  86 +++++++++++----------------
> > 	 lib/seq_buf.c           | 101 ++++++++++----------------------
> > 	 2 files changed, 66 insertions(+), 121 deletions(-)
> > 
> > (It's far from ready.)
> 
> Don't bother. I don't think you understand the usage of seq_buf.

I'm certainly not understanding all the use cases of it.  I'm just
seeing it being used to replace snprintf(3) and strscpy(9) calls, in
theory to make them safer, and I'm doubting that will really improve
safety.  For this use case, a pointer-based API is much safer and
simpler, and I'd entirely avoid seq_buf.  That's why I'm trying to learn
seq_buf and see if it can be changed in that way.


Cheers,
Alex

> I also
> already don't like the changes and I don't see why they are even necessary.
> 
> -- Steve
> 

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

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