On Tue, Feb 27, 2024 at 10:38:10AM +0100, Rasmus Villemoes wrote:
> On 26/02/2024 15.57, Jani Nikula wrote:
>
> > Personally I suck at remembering even the standard printf conversion
> > specifiers, let alone all the kernel extensions. I basically have to
> > look them up every time. I'd really love some %{name} format for named
> > pointer things. And indeed preferrably without the %p. Just %{name}.
>
> Sorry to spoil the fun, but that's a non-starter.
>
> foo.c: In function ‘foo’:
> foo.c:5:24: warning: unknown conversion type character ‘{’ in format
> [-Wformat=]
> 5 | printf("Hello %{function} World\n", &foo);
> | ^
>
> You can't start accepting stuff that -Wformat will warn about. We're not
> going to start building with Wno-format.
Are there any sensible looking characters we could use for
this? Ideally I'd like to have something to bracket the
outsides, and perhaps a namespace separator in the middle.
Or are we really forced into having essentially a random set
of characters following just a %p/etc.?
>
> > And then we could discuss adding support for drm specific things. I
> > guess one downside is that the functions to do this would have to be in
> > vsprintf.c instead of drm. Unless we add some code in drm for this
> > that's always built-in.
>
> If people can be trusted to write callbacks with the proper semantics
> for snprintf [1], we could do a generic
Yeah, I was at some point thinking that having a version of
register_printf_function() for printk() might be nice. The dangers
being that we get conflicts between subsystems (*), or that it gets
totally out of hand, or as you point out below people will start
to do questionable things in there.
(*) My earlier "include a subsystem namespace in the format"
idea was basically how I was thinking of avoiding conflicts.
>
> typedef char * (*printf_callback)(char *buf, char *end, void *ctx);
>
> struct printf_ext {
> printf_callback cb;
> void *ctx;
> };
>
> #define PRINTF_EXT(callback, context) &(struct printf_ext){ .cb =
> callback, .ctx = context }
>
> // in drm-land
>
> char* my_drm_gizmo_formatter(char *buf, char *end, void *ctx)
> {
> struct drm_gizmo *dg = ctx;
> ....
> return buf;
> }
> #define pX_gizmo(dg) PRINTF_EXT(my_drm_gizmo_formatter, dg)
>
> printk("error: gizmo %pX in wrong state!\n", pX_gizmo(dg));
>
> Then vsprintf.c doesn't need to know anything about any particular
> subsystem. And if a subsystem breaks snprintf semantics, they get to
> keep the pieces. With a little more macro magic, one might even be able
> to throw in some type safety checks.
>
> Rasmus
>
> [1] You can't sleep, you can't allocate memory, you probably can't even
> take any raw spinlocks, you must attempt to do the full formatting so
> you can tell how much room would be needed, but you must of course not
> write anything beyond end. Calling vsnprintf() to format various integer
> members is probably ok, but recursively using %pX to print full
> subobjects is likely a bad idea.
--
Ville Syrjälä
Intel
