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
