Hi Steve, > Date: 2026-10-29 09:41:56-0400 > From: Steven Rostedt > > On Thu, 29 Oct 2026 12:33:19 +0100 > Alejandro Colomar 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); > > } > --