[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <svune35mcrnaiuoz4xtzegnghojmphxulpb2jdgczy3tcqaijb@dskdebhbhtrq>
Date: Thu, 10 Jul 2025 20:30:52 +0200
From: Alejandro Colomar <alx@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-mm@...ck.org, linux-hardening@...r.kernel.org,
Kees Cook <kees@...nel.org>, Christopher Bazley <chris.bazley.wg14@...il.com>,
shadow <~hallyn/shadow@...ts.sr.ht>, linux-kernel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>, kasan-dev@...glegroups.com, Dmitry Vyukov <dvyukov@...gle.com>,
Alexander Potapenko <glider@...gle.com>, Marco Elver <elver@...gle.com>, Christoph Lameter <cl@...ux.com>,
David Rientjes <rientjes@...gle.com>, Vlastimil Babka <vbabka@...e.cz>,
Roman Gushchin <roman.gushchin@...ux.dev>, Harry Yoo <harry.yoo@...cle.com>,
Andrew Clayton <andrew@...ital-domain.net>, Rasmus Villemoes <linux@...musvillemoes.dk>,
Michal Hocko <mhocko@...e.com>, Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [RFC v4 6/7] sprintf: Add [V]SPRINTF_END()
Hi Linus,
On Thu, Jul 10, 2025 at 08:52:13AM -0700, Linus Torvalds wrote:
> On Wed, 9 Jul 2025 at 19:49, Alejandro Colomar <alx@...nel.org> wrote:
> >
> > +#define SPRINTF_END(a, fmt, ...) sprintf_end(a, ENDOF(a), fmt, ##__VA_ARGS__)
> > +#define VSPRINTF_END(a, fmt, ap) vsprintf_end(a, ENDOF(a), fmt, ap)
>
> So I like vsprintf_end() more as a name ("like more" not being "I love
> it", but at least it makes me think it's a bit more self-explanatory).
:-)
> But I don't love screaming macros. They historically scream because
> they are unsafe, but they shouldn't be unsafe in the first place.
>
> And I don't think those [V]SPRINTF_END() and ENDOF() macros are unsafe
> - they use our ARRAY_SIZE() macro which does not evaluate the
> argument, only the type, and is safe to use.
Yup, it's safe to use.
> So honestly, this interface looks easy to use, but the screaming must stop.
>
> And none of this has *anything* to do with "end" in this form anyway.
That same thing happened through my head while doing it, but I didn't
think of a better name.
In shadow, we have many interfaces for which we have an uppercase macro
version of many functions that gets array sizes and other extra safety
measures where we can. (So there, the uppercase versions are indeed
extra safety, instead of the historical "there be dragons". I use the
uppercase to mean "this does some magic to be safer".)
> IOW, why isn't this just
>
> #define sprintf_array(a,...) snprintf(a, ARRAY_SIZE(a), __VA_ARGS__)
Agree. This is a better name for the kernel.
> which is simpler and more direct, doesn't use the "end" version that
> is pointless (it's _literally_ about the size of the array, so
> 'snprintf' is the right thing to use),
I disagree with snprintf(3), but not because of the input, but rather
because of the output. I think an API similar to strscpy() would be
better, so it can return an error code for truncation. In fact, up to
v2, I had a stprintf() (T for truncation) that did exactly that.
However, I found out I could do the same with sprintf_end(), which would
mean one less function to grok, which is why I dropped that part.
I'll use your suggested name, as I like it. Expect v5 in a few minutes.
> doesn't scream, and has a
> rather self-explanatory name.
>
> Naming matters.
+1
Have a lovely day!
Alex
>
> Linus
--
<https://www.alejandro-colomar.es/>
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists