linux-kernel - Re: [PATCH v2 00/28] Printbufs (now with more printbufs!)

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: <Yo+SRBSu5OdRl0/J@alley>
Date:   Thu, 26 May 2022 16:44:20 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Kent Overstreet <kent.overstreet@...il.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 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