lkml.org 
[lkml]   [2022]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 00/28] Printbufs (now with more printbufs!)
On Thu, May 26, 2022 at 04:44:20PM +0200, Petr Mladek wrote:
> On Thu 2022-05-19 13:23:53, Kent Overstreet wrote:
> > So there's a lot of new stuff since the first posting:
>
> > - Printbufs have been broken up into multiple patches that each add distinct
> > functionality - this is intended to make it easier to review and to see
> > what's used for what
>
> It is great that it is split. Also it is great to see all the ideas.
> But I would really prefer to somehow split it to make it easier
> for review and rebasing.
>
> I see the following "independent" parts:
>
> 1. Add simple API that allows to replace @len, @buf, @end in vsprintf.c
> by @printbuf. I agree that the code looks better and more safe.
>
> 2. Clean up of printf_spec. It would be great. But I do not like
> some parts. For example, si_units, human_readable_units
> are not property of the buffer. They are specific for a
> particular substring.

Not in conventional usage - these are properties that are set globally when
building up a string: consider an -h flag to a userspace utility.

>
> 3. New %p(%p) format. It really needs deep thinking. It is a
> ticket for potential big troubles. It is one patch that
> might be introduced and discussed anytime once we have
> the simple buffer API.

It's split out into a separate patch - discuss away!

> 3. Replace seq_buf. Steven Rostedt has to agree with it. Honestly,
> I do not see any improvement. The patches mostly do 1:1 replacement
> of one API with another.

It was necessary to avoid code duplication, which Christoph didn't like, and
seq_buf wasn't quite right for what I was trying to do and Steven didn't want to
change it. (read_pos is tracing specific and doesn't have anything to do with
printing, some of the semantics had to be tweaked to support snprintf, and the
name was wrong... :)

> 4. Heap allocated buffer. I am not sure if it is really needed.
> The patchset adds 3 users. IMHO, small static buffer would be
> perfectly fine for 2 of them. I personally do not like the error
> handling and the need to call exit.
>
> 5. All the fancy stuff (pr_tab(), pr_string_option()). The patchset
> does not add any user for them.

Heap allocated buffers and tabstops: these are things that I've been using in
bcachefs, and have quickly become necessities - I included them because I think
others will soon find them valuable (e.g. /proc has a lot of files that would be
more readable if formatted with tabstops).

pr_string_option(): this isn't anything really new, we've got a lot of pretty
printers and string helpers of this nature that need to be better organized and
given a common calling convention, I included this as code I've got that's been
useful and I think does it cleanly. I have more future plans for pretty printer
cleanup.

\
 
 \ /
  Last update: 2022-05-26 17:12    [W:0.295 / U:0.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site