Re: [PATCH v2] seq_buf: Introduce DECLARE_SEQ_BUF and seq_buf_str()
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2023-10-26 20:20:30
Also in:
linux-hardening, lkml
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2023-10-26 20:20:30
Also in:
linux-hardening, lkml
On Thu, Oct 26, 2023 at 12:40:37PM -0700, Kees Cook wrote:
Solve two ergonomic issues with struct seq_buf; 1) Too much boilerplate is required to initialize: struct seq_buf s; char buf[32]; seq_buf_init(s, buf, sizeof(buf)); Instead, we can build this directly on the stack. Provide DECLARE_SEQ_BUF() macro to do this: DECLARE_SEQ_BUF(s, 32); 2) %NUL termination is fragile and requires 2 steps to get a valid C String (and is a layering violation exposing the "internals" of seq_buf): seq_buf_terminate(s); do_something(s->buffer); Instead, we can just return s->buffer direction after terminating it in refactored seq_buf_terminate(), now known as seq_buf_str(): do_soemthing(seq_buf_str(s));
...
+#define DECLARE_SEQ_BUF(NAME, SIZE) \
+ char __ ## NAME ## _buffer[SIZE] = ""; \
+ struct seq_buf NAME = { .buffer = &__ ## NAME ## _buffer, \
+ .size = SIZE }
Hmm... Wouldn't be more readable to have it as
#define DECLARE_SEQ_BUF(NAME, SIZE) \
char __ ## NAME ## _buffer[SIZE] = ""; \
struct seq_buf NAME = { \
.buffer = &__ ## NAME ## _buffer, \
.size = SIZE, \
}
?
...
+static inline char *seq_buf_str(struct seq_buf *s)
{
if (WARN_ON(s->size == 0))
- return;
+ return "";I'm wondering why it's a problem to have an empty string?
if (seq_buf_buffer_left(s)) s->buffer[s->len] = 0; else s->buffer[s->size - 1] = 0; + + return s->buffer; }
-- With Best Regards, Andy Shevchenko