Messages in this thread | | | Date | Thu, 26 May 2022 11:21:27 -0400 | From | Kent Overstreet <> | Subject | Re: [PATCH v2 01/28] lib/printbuf: New data structure for printing strings |
| |
On Thu, May 26, 2022 at 05:06:15PM +0200, Petr Mladek wrote: > On Thu 2022-05-19 13:23:54, Kent Overstreet wrote: > > This adds printbufs: a printbuf points to a char * buffer and knows the > > size of the output buffer as well as the current output position. > > > > Future patches will be adding more features to printbuf, but initially > > printbufs are targeted at refactoring and improving our existing code in > > lib/vsprintf.c - so this initial printbuf patch has the features > > required for that. > > > diff --git a/include/linux/printbuf.h b/include/linux/printbuf.h > > new file mode 100644 > > index 0000000000..40dc07040d > > --- /dev/null > > +++ b/include/linux/printbuf.h > > +static inline void pr_chars(struct printbuf *out, char c, unsigned n) > > +{ > > + memset(out->buf + out->pos, > > + c, > > + min(n, printbuf_remaining(out))); > > + out->pos += n; > > + printbuf_nul_terminate(out); > > +} > > This function is not later used. Please, do not add API > that will not have users in the same patchset. > > There are several other cases. I am not going to comment > all of them.
It is used in this patchset, in lib/vsnprintf.c. You sure about the other cases?
> > +static inline void __pr_char(struct printbuf *out, char c) > > +{ > > + if (printbuf_remaining(out)) > > + out->buf[out->pos] = c; > > + out->pos++; > > +} > > + > > +static inline void pr_char(struct printbuf *out, char c) > > +{ > > + __pr_char(out, c); > > + printbuf_nul_terminate(out); > > +} > > The "pr_" prefix is a nightmare for me because the same prefix > is used also for printk() API ;-) > > Could we please use "pb_" instead?
I'm not entirely against that, but I see printbufs as already in this patchset tightly coupled to vsprintf.c and thus quite related to printk, as well - and there aren't that many different pr_ things. So I think the shared prefix makes some sense, I'd like to hear what others think before making that change.
| |