Received-SPF: pass (google.com: domain of linux-kernel+bounces-82970-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <1013ff2d-76b2-41f9-a5d4-39a567a3b0cc@rasmusvillemoes.dk>
Date: Tue, 27 Feb 2024 10:38:10 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 01/12] drm/i915: Indicate which pipe failed the fastset
 check overall
Content-Language: en-US, da
To: Jani Nikula <jani.nikula@linux.intel.com>,
 =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= <ville.syrjala@linux.intel.com>,
 Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org, Petr Mladek <pmladek@suse.com>,
 Steven Rostedt <rostedt@goodmis.org>,
 Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
 Sergey Senozhatsky <senozhatsky@chromium.org>, linux-kernel@vger.kernel.org
References: <20240215164055.30585-1-ville.syrjala@linux.intel.com>
 <20240215164055.30585-2-ville.syrjala@linux.intel.com>
 <ZdfApN1h97GTfL1t@intel.com> <Zdj2ONs8BZ6959Xb@intel.com>
 <87bk83mfwp.fsf@intel.com>
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
In-Reply-To: <87bk83mfwp.fsf@intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

> 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

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.