linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Joe Perches <joe@perches.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Matthew Wilcox <willy@infradead.org>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] vsprintf/Documentation: Add X to %*ph extension to output upper case hex
Date: Fri, 27 Aug 2021 12:49:03 +0200	[thread overview]
Message-ID: <YSjDH1y8AJIvCstV@alley> (raw)
In-Reply-To: <YSinAyVMIQDxf6Fo@smile.fi.intel.com>

On Fri 2021-08-27 11:49:07, Andy Shevchenko wrote:
> On Fri, Aug 27, 2021 at 01:08:10AM -0700, Joe Perches wrote:
> > On Fri, 2021-08-27 at 10:48 +0300, Andy Shevchenko wrote:
> > > On Thu, Aug 26, 2021 at 11:43:01AM -0700, Joe Perches wrote:
> > > > A few sysfs output uses of hex arrays are uppercase and are nominally ABI.
> > > > 
> > > > Add a mechanism to the existing vsprintf %*ph hex output extension to
> > > > support upper case hex output.
> > > 
> > > ...
> > > 
> > > > +	The preferred output is lowercase
> > > >  	%*ph	00 01 02  ...  3f
> > > >  	%*phC	00:01:02: ... :3f
> > > >  	%*phD	00-01-02- ... -3f
> > > >  	%*phN	000102 ... 3f
> > > > +	Formats with X are uppercase, used for backwards compatibility
> > > > +	%*phX	00 01 02  ...  3F
> > > > +	%*phCX	00:01:02: ... :3F
> > > > +	%*phDX	00-01-02- ... -3F
> > > > +	%*phNX	000102 ... 3F
> > > 
> > > Why not using %*pH...?

I though about this as well.

> > I find X more intelligible.

I would slightly prefer %pH. I always have problems to parse long
sequences of modifiers. So, the shorter format the better.

Of course, it means that 'H' won't be usable for another purpose.
But it will happen one day anyway. Well, this is why I do not
have strong opinion.

I am more and more convinced that we will need another approach.
Mathew Wilcox has had an idea to add support for custom callbacks
that would be able to format the string, something like:

   vsprintf("Date: %pX(%p)\n", format_date, time_stamp);

I think that it might even be possible to do something like:

   vsprintf("Date: %pX\n", format_date(time));

, where the format_date() would be a macro that would create
a struct at stack a pass it as a pointer:

#define format_date(time)			   \
({						   \
	struct vsprintf_callback c = {		   \
		.func = vsprintf_format_date,	   \
		.arg1 = time,			   \
	}					   \
						   \
	&c;					   \
})

and vsprintf would internally do something like:

char *custom_format(char *buf, char *end, vsprintf_callback *c,
			 struct printf_spec spec, const char *fmt)
{
	return c->func(buf, end, c->arg1, spec);
}

It would allow to replace all the magic %pXYZ modifiers with
self-explanatory callbacks. While still keeping it easy to use.

Best Regards,
Petr

  reply	other threads:[~2021-08-27 10:49 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 18:43 [PATCH 0/5] vsprintf and uses: Add upper case output to %*ph extension Joe Perches
2021-08-26 18:43 ` [PATCH 1/5] vsprintf/Documentation: Add X to %*ph extension to output upper case hex Joe Perches
2021-08-27  7:48   ` Andy Shevchenko
2021-08-27  8:08     ` Joe Perches
2021-08-27  8:49       ` Andy Shevchenko
2021-08-27 10:49         ` Petr Mladek [this message]
2021-08-28  2:49         ` Joe Perches
2021-08-26 18:43 ` [PATCH 2/5] scsi: aacraid: Use vsprintf %phNX extension Joe Perches
2021-08-26 18:43 ` [PATCH 3/5] scsi: hpsa: " Joe Perches
2021-08-26 18:43 ` [PATCH 4/5] scsi: smartpqi: " Joe Perches
2021-08-26 18:43 ` [PATCH 5/5] staging: r8188eu: Use vsprintf extension %phCX to format a copy_to_user string Joe Perches
2021-08-27  8:42   ` Greg Kroah-Hartman
2021-08-27 15:23     ` Joe Perches
2021-08-27 15:27       ` Greg Kroah-Hartman
2021-08-27 15:52         ` Joe Perches
2021-08-27  7:51 ` [PATCH 0/5] vsprintf and uses: Add upper case output to %*ph extension Andy Shevchenko
2021-08-27  8:10   ` Joe Perches
2021-08-27  8:46     ` Andy Shevchenko
2021-08-27 10:23       ` Greg KH
2021-08-27 16:09         ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YSjDH1y8AJIvCstV@alley \
    --to=pmladek@suse.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=corbet@lwn.net \
    --cc=joe@perches.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).