linux-kernel - Re: [PATCH v2 01/28] lib/printbuf: New data structure for printing strings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220529181558.pg4knymlixphra5i@moria.home.lan>
Date:   Sun, 29 May 2022 14:15:58 -0400
From:   Kent Overstreet <kent.overstreet@...il.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...r.kernel.org,
        rostedt@...dmis.org, senozhatsky@...omium.org,
        andriy.shevchenko@...ux.intel.com, willy@...radead.org
Subject: Re: [PATCH v2 01/28] lib/printbuf: New data structure for printing
 strings

On Fri, May 27, 2022 at 12:29:20PM +0200, Petr Mladek wrote:
> I would really like to keep the three APIs separated and easy to
> distinguish. They are principally different:
> 
> 1. pr_*() API:
> 
>        + wrapper to printk(). They makes the messages available on
> 	 console and for user-space log daemons while printf()
> 
>       + the various pr_*() variants are used to define kernel
> 	specific features and behavior, for example:
> 	loglevel, ratelimit, only once. deferred console handling.
> 
>        + uses implicit (system) buffer
> 
>        + The message format is defined by the 1st parameter. It
> 	 is the same way as printf() in user-space.
> 
>        + It is inspired by printf() from user-space that prints
> 	 the messages to the standard output.
> 
> 
> 2. *s*printf() APIs:
> 
>        + basically duplicate the same user-space API. It supports
> 	 some extra %p modifiers. There might be few more
> 	 incompatibilities.
> 
>        + use simple "char *" buffer provided as the 1st parameter
> 
>        + the messages format is defined the same way as in
> 	 the user-space counterparts.

After printbufs are merged, I think we should consider formally deprecating
sprintf/snprintf, certainly for new code. As you saw from the vsprintf.c
cleanup, printbufs are _much_ nicer than passing around char */length - it's
2022, we shouldn't be doing that anymore!

> 
> 
> 3. printbuf API:
> 
>        + append messages into the given printbuf by small pieces
> 
>        + format defined by the suffix, for example, _char(),
> 	 bytes(), units_64(), _tab(), indent()
> 
>        + allows to do special operations on the buffer,
> 	 for example, _reset(), make_room(), atomic_inc()

atomic_inc() should not exist in the long term - we _really_ need
memalloc_nowait_(save|restore), that's the correct way to do this.

>        + it will be used as low-level API for vscnprinf()
> 	 implementation, pretty printing API, or
> 	 stand alone uses.
> 
>        + I wonder if there will be variant that will allow
> 	 to pass the format in the printf() way, e.g.
> 	 int pb_printf(printbuf *buf, const char *fmt, ...);

That's pr_buf()/vpr_buf(), and I heavily use pr_buf() in my own code.

snprintf() is just a wrapper around pr_buf() now.

>        + is there any user space counter part?

I've been using the previous version of this code in userspace that was part of
bcachefs, and my intention is very much for this code to also be used in
userspace as well.

Bringing the base printbuf API to userspace is trivial - i.e. doing it as a
wrapper around snprintf(), which is how printbufs started. However, the %(%p)
format string extension for calling pretty-printers directly - which I badly
want and think is far superior to what glibc has [1], will also require patching
glibc (and gcc, to get the format string that we want).

So that'll be a little ways off.

> Now, it is clear that printfbuf API must be distinguished by another
> prefix:
> 
>        + it must be clear that it stores the output into printbuf.
> 	 It is similar to dprintf(), fprintf(), sprintf().
> 
>        + It can't be done by the suffix because it is already used
> 	 to define format of the appended string or extra operation.
> 
>        + It must be clear what is low-level API used to implement
> 	 vsprintf() and high-level API that uses vsprintf().
> 	 I mean pb_char() vs. pb_printf().

I'm coming around to the pb_* naming idea. pbprintf() doesn't roll off the
tongue in the same way that pr_buf() does, but I guess I can live with that.

1: https://www.gnu.org/software/libc/manual/html_node/Printf-Extension-Example.html