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 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.

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.

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.

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.


I am going to comment the particular patches. It might take some
time. The patchset is really huge. It would really help to split it.

Best Regards,
Petr

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