Re: [RFC v3 3/7] mm: Use seprintf() instead of less ergonomic APIs

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3993 bytes --]

Hi Linus,

On Mon, Jul 07, 2025 at 12:17:11PM -0700, Linus Torvalds wrote:
> On Sun, 6 Jul 2025 at 22:06, Alejandro Colomar <alx@kernel.org> wrote:
> >
> > -       p += snprintf(p, ID_STR_LENGTH - (p - name), "%07u", s->size);
> > +       p = seprintf(p, e, "%07u", s->size);
> 
> I am *really* not a fan of introducing yet another random non-standard
> string function.

I am in the C Committee, and have proposed this API for standardization.
I have a feeling that the committee might be open to it.

> This 'seprintf' thing really seems to be a completely made-up thing.
> Let's not go there. It just adds more confusion - it may be a simpler
> interface, but it's another cogniitive load thing,

I understand the part of your concern that relates to
<https://xkcd.com/927/>.

However, I've shown how in mm/, I got rid of most snprintf() and
scnprintf() calls.  I could even get rid of the remaining snprintf()
ones; I didn't do it to avoid churn, but they're just 3, so I could do
it, as a way to remove all uses of snprintf(3).

I also got rid of all scnprintf() uses except for 2.  Not because those
two cannot be removed, but because the code was scary enough that I
didn't dare touch it.  I'd like someone to read it and confirm that it
can be replaced.

> and honestly, that
> "beginning and end" interface is not great.

Just look at the diffs.  It is great, in terms of writing less code.

In some cases, it makes sense to pass a size.  Those cases are when you
don't want to chain several calls.  That's the case of stprintf(), and
it's wrapper STPRINTF(), which calls ARRAY_SIZE() internally.

But most of the time you want to chain calls, and 'end' beats 'size'
there.

> I think we'd be better off with real "character buffer" interfaces,
> and they should be *named* that way, not be yet another "random
> character added to the printf family".

You might want to do that, but I doubt it's an easy change.  On the
other hand, this change is trivial, and can be done incrementally,
without needing to modify the buffer since its inception.

And you can come back later to wrap this in some API that does what you
want.  Nothing stops you from doing that.

But this fixes several cases of UB in a few files that I've looked at,
with minimal diffs.

> The whole "add a random character" thing is a disease. But at least
> with printf/fprintf/vprintf/vsnprintf/etc, it's a _standard_ disease,
> so people hopefully know about it.

seprint(2) was implemented in Plan9 many decades ago.  It's not
standard, because somehow Plan9 has been ignored by history, but the
name has a long history.

<https://plan9.io/magic/man2html/2/print>

Plus, I'm making seprintf() standard (if I can convince the committee).

Yesterday night, I presented the proposal to the committee, informally
(via email).  You can read a copy here:
<https://lore.kernel.org/linux-hardening/cover.1751747518.git.alx@kernel.org/T/#m9311035d60b4595db62273844d16671601e77a50>

I'll present it formally in a month, since I have a batch of proposals
for the committee in the works.


Have a lovely day!
Alex

> So I really *really* don't like things like seprintf(). It just makes me go WTF?
> 
> Interfaces that have worked for us are things like "seq_printf()", which
> 
>  (a) has sane naming, not "add random characters"
> 
>  (b) has real abstractions (in that case 'struct seq_file') rather
> than adding random extra arguments to the argument list.
> 
> and we do have something like that in 'struct seq_buf'.  I'm not
> convinced that's the optimal interface, but I think it's *better*.
> Because it does both encapsulate a proper "this is my buffer" type,
> and has a proper "this is a buffer operation" function name.
> 
> So I'd *much* rather people would try to convert their uses to things
> like that, than add random letter combinations.
> 
>              Linus

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]