diff mbox

[v1,04/25] lib/vsprintf: Print time and date in human readable format via %pt

Message ID 20170608134811.60786-5-andriy.shevchenko@linux.intel.com
State Changes Requested
Headers show

Commit Message

Andy Shevchenko June 8, 2017, 1:47 p.m. UTC
There are users which print time and date represented by content of
struct rtc_time in human readable format.

Instead of open coding that each time introduce %pt[dt][rv] specifier.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jason Wessel <jason.wessel@windriver.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/printk-formats.txt |  17 ++++++
 lib/vsprintf.c                   | 124 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 141 insertions(+)

Comments

Arnd Bergmann June 8, 2017, 2:49 p.m. UTC | #1
On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> There are users which print time and date represented by content of
> struct rtc_time in human readable format.
>
> Instead of open coding that each time introduce %pt[dt][rv] specifier.

I really like the idea, and the implementation seems fine for this use case, but
before we reserve %pt for rtc_time, could we discuss whether we want
that for printing struct tm, struct timespec64, time64_t or ktime_t instead?

I can see good reasons for pretty-printing any of them, but the namespace for
format strings is rather limited.

struct rtc_time is almost the same as struct tm (the former has one extra
member), so maybe we can actually define them to be the same and
use one format string for both?

        Arnd
Andy Shevchenko June 8, 2017, 2:55 p.m. UTC | #2
On Thu, Jun 8, 2017 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> There are users which print time and date represented by content of
>> struct rtc_time in human readable format.
>>
>> Instead of open coding that each time introduce %pt[dt][rv] specifier.
>
> I really like the idea, and the implementation seems fine for this use case, but
> before we reserve %pt for rtc_time, could we discuss whether we want
> that for printing struct tm, struct timespec64, time64_t or ktime_t instead?

How many users? For struct tm it's somelike 4 (which want to print its content).

> I can see good reasons for pretty-printing any of them, but the namespace for
> format strings is rather limited.
>
> struct rtc_time is almost the same as struct tm (the former has one extra
> member), so maybe we can actually define them to be the same and
> use one format string for both?

The reason I decide to drop struct tm for now due to they are not
compatible and I have got an interesting bugs.
Verify tm_year member carefully.
Alexandre Belloni June 8, 2017, 3:05 p.m. UTC | #3
On 08/06/2017 at 17:55:12 +0300, Andy Shevchenko wrote:
> On Thu, Jun 8, 2017 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> >> There are users which print time and date represented by content of
> >> struct rtc_time in human readable format.
> >>
> >> Instead of open coding that each time introduce %pt[dt][rv] specifier.
> >
> > I really like the idea, and the implementation seems fine for this use case, but
> > before we reserve %pt for rtc_time, could we discuss whether we want
> > that for printing struct tm, struct timespec64, time64_t or ktime_t instead?
> 
> How many users? For struct tm it's somelike 4 (which want to print its content).
> 
> > I can see good reasons for pretty-printing any of them, but the namespace for
> > format strings is rather limited.
> >
> > struct rtc_time is almost the same as struct tm (the former has one extra
> > member), so maybe we can actually define them to be the same and
> > use one format string for both?
> 
> The reason I decide to drop struct tm for now due to they are not
> compatible and I have got an interesting bugs.
> Verify tm_year member carefully.
> 

I understand this may not fit your debugging needs but what about pretty
printing time64_t and using rtc_tm_to_time64?
Arnd Bergmann June 8, 2017, 3:33 p.m. UTC | #4
On Thu, Jun 8, 2017 at 4:55 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Jun 8, 2017 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>> There are users which print time and date represented by content of
>>> struct rtc_time in human readable format.
>>>
>>> Instead of open coding that each time introduce %pt[dt][rv] specifier.
>>
>> I really like the idea, and the implementation seems fine for this use case, but
>> before we reserve %pt for rtc_time, could we discuss whether we want
>> that for printing struct tm, struct timespec64, time64_t or ktime_t instead?
>
> How many users?

It's hard to predict, I would assume we get more users once there is an
easy way to print the time.

> For struct tm it's somelike 4 (which want to print its content).

Good point. I notice that they all convert from time64_t or time_t into
struct tm immediately before printing it, so we can scratch that one
as long as there is a way to pretty-print a time64_t. We also don't
need to print a time_t as we want to kill that one off anyway.

If we only care about printing time64_t and rtc_time, we can easily
use %pT for one and %pt for the other, but there may still be good
reasons to print a timespec64 or ktime_t.

>> I can see good reasons for pretty-printing any of them, but the namespace for
>> format strings is rather limited.
>>
>> struct rtc_time is almost the same as struct tm (the former has one extra
>> member), so maybe we can actually define them to be the same and
>> use one format string for both?
>
> The reason I decide to drop struct tm for now due to they are not
> compatible and I have got an interesting bugs.

Ok.

        Arnd
Joe Perches June 8, 2017, 3:48 p.m. UTC | #5
On Thu, 2017-06-08 at 17:33 +0200, Arnd Bergmann wrote:
> On Thu, Jun 8, 2017 at 4:55 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jun 8, 2017 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > There are users which print time and date represented by content of
> > > > struct rtc_time in human readable format.
> > > > 
> > > > Instead of open coding that each time introduce %pt[dt][rv] specifier.
> > > 
> > > I really like the idea, and the implementation seems fine for this use case, but
> > > before we reserve %pt for rtc_time, could we discuss whether we want
> > > that for printing struct tm, struct timespec64, time64_t or ktime_t instead?
> > 
> > How many users?
> 
> It's hard to predict, I would assume we get more users once there is an
> easy way to print the time.
> 
> > For struct tm it's somelike 4 (which want to print its content).
> 
> Good point. I notice that they all convert from time64_t or time_t into
> struct tm immediately before printing it, so we can scratch that one
> as long as there is a way to pretty-print a time64_t. We also don't
> need to print a time_t as we want to kill that one off anyway.
> 
> If we only care about printing time64_t and rtc_time, we can easily
> use %pT for one and %pt for the other, but there may still be good
> reasons to print a timespec64 or ktime_t.

> > > I can see good reasons for pretty-printing any of them, but the namespace for
> > > format strings is rather limited.


The kernel already uses different types for the same leading
letter.  For
instance: %pI4 vs %pI6, %pap vs %pad

A single 't' letter could do reasonably well.
Andy Shevchenko June 8, 2017, 5:57 p.m. UTC | #6
On Thu, Jun 8, 2017 at 6:05 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 08/06/2017 at 17:55:12 +0300, Andy Shevchenko wrote:
>> On Thu, Jun 8, 2017 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
>> > <andriy.shevchenko@linux.intel.com> wrote:
>> >> There are users which print time and date represented by content of
>> >> struct rtc_time in human readable format.
>> >>
>> >> Instead of open coding that each time introduce %pt[dt][rv] specifier.
>> >
>> > I really like the idea, and the implementation seems fine for this use case, but
>> > before we reserve %pt for rtc_time, could we discuss whether we want
>> > that for printing struct tm, struct timespec64, time64_t or ktime_t instead?
>>
>> How many users? For struct tm it's somelike 4 (which want to print its content).
>>
>> > I can see good reasons for pretty-printing any of them, but the namespace for
>> > format strings is rather limited.
>> >
>> > struct rtc_time is almost the same as struct tm (the former has one extra
>> > member), so maybe we can actually define them to be the same and
>> > use one format string for both?
>>
>> The reason I decide to drop struct tm for now due to they are not
>> compatible and I have got an interesting bugs.
>> Verify tm_year member carefully.
>>
>
> I understand this may not fit your debugging needs but what about pretty
> printing time64_t and using rtc_tm_to_time64?

There are two downsides as I can see:
1) conversion to and from just for that;
2) if you look closer to the patches rtc-* you may find cases where
wday is also printed so, struct rtc_time still will be in use.

So, I would go not to  convert if there is no strong reason to do.
Andy Shevchenko June 8, 2017, 6:02 p.m. UTC | #7
On Thu, Jun 8, 2017 at 6:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Jun 8, 2017 at 4:55 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Thu, Jun 8, 2017 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
>>> <andriy.shevchenko@linux.intel.com> wrote:

>>> I really like the idea, and the implementation seems fine for this use case, but
>>> before we reserve %pt for rtc_time, could we discuss whether we want
>>> that for printing struct tm, struct timespec64, time64_t or ktime_t instead?
>>
>> How many users?
>
> It's hard to predict, I would assume we get more users once there is an
> easy way to print the time.

So, at least for now we can guess using existing users, right?

I don't check yet how to calculate those cases of time64_t,
timespec64, ktime_t and alike if they are about pretty ptintong time
and date.
I'm speculating that there are (almost) none.

>> For struct tm it's somelike 4 (which want to print its content).
>
> Good point. I notice that they all convert from time64_t or time_t into
> struct tm immediately before printing it, so we can scratch that one
> as long as there is a way to pretty-print a time64_t. We also don't
> need to print a time_t as we want to kill that one off anyway.
>
> If we only care about printing time64_t and rtc_time, we can easily
> use %pT for one and %pt for the other, but there may still be good
> reasons to print a timespec64 or ktime_t.

No need, we may still use 3rd/4th letter in the format for that.

%pt(t/d) time/date + whatever modifications, like raw, validate, timespec, etc.

's' for timespec64, for example.
Alexandre Belloni June 8, 2017, 6:41 p.m. UTC | #8
On 08/06/2017 at 20:57:05 +0300, Andy Shevchenko wrote:
> On Thu, Jun 8, 2017 at 6:05 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
> > On 08/06/2017 at 17:55:12 +0300, Andy Shevchenko wrote:
> >> On Thu, Jun 8, 2017 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
> >> > <andriy.shevchenko@linux.intel.com> wrote:
> >> >> There are users which print time and date represented by content of
> >> >> struct rtc_time in human readable format.
> >> >>
> >> >> Instead of open coding that each time introduce %pt[dt][rv] specifier.
> >> >
> >> > I really like the idea, and the implementation seems fine for this use case, but
> >> > before we reserve %pt for rtc_time, could we discuss whether we want
> >> > that for printing struct tm, struct timespec64, time64_t or ktime_t instead?
> >>
> >> How many users? For struct tm it's somelike 4 (which want to print its content).
> >>
> >> > I can see good reasons for pretty-printing any of them, but the namespace for
> >> > format strings is rather limited.
> >> >
> >> > struct rtc_time is almost the same as struct tm (the former has one extra
> >> > member), so maybe we can actually define them to be the same and
> >> > use one format string for both?
> >>
> >> The reason I decide to drop struct tm for now due to they are not
> >> compatible and I have got an interesting bugs.
> >> Verify tm_year member carefully.
> >>
> >
> > I understand this may not fit your debugging needs but what about pretty
> > printing time64_t and using rtc_tm_to_time64?
> 
> There are two downsides as I can see:
> 1) conversion to and from just for that;

Those are almost all debug messages, I would be fine with that.

> 2) if you look closer to the patches rtc-* you may find cases where
> wday is also printed so, struct rtc_time still will be in use.
> 

(And you missed two in rtc-mcp795.c). Honestly, nobody cares about wday,
you may as well leave it out.
Andy Shevchenko June 8, 2017, 6:49 p.m. UTC | #9
On Thu, Jun 8, 2017 at 9:41 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 08/06/2017 at 20:57:05 +0300, Andy Shevchenko wrote:
>> On Thu, Jun 8, 2017 at 6:05 PM, Alexandre Belloni
>> <alexandre.belloni@free-electrons.com> wrote:

>> > I understand this may not fit your debugging needs but what about pretty
>> > printing time64_t and using rtc_tm_to_time64?
>>
>> There are two downsides as I can see:
>> 1) conversion to and from just for that;
>
> Those are almost all debug messages, I would be fine with that.

Yeah, but the problem is to pass the reference. All dances around will
uglify the code.
(Obviously we can't pass timespec64/time64_t or anything longer than
32 bits as is in %p extension)

>> 2) if you look closer to the patches rtc-* you may find cases where
>> wday is also printed so, struct rtc_time still will be in use.

> (And you missed two in rtc-mcp795.c). Honestly, nobody cares about wday,
> you may as well leave it out.

Oops, thanks, indeed. Okay, I will leave it for now with dropped wday
until someone comes with strong opinion why it should be there.
Rasmus Villemoes June 8, 2017, 8:42 p.m. UTC | #10
On Thu, Jun 08 2017, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, Jun 8, 2017 at 9:41 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
>> On 08/06/2017 at 20:57:05 +0300, Andy Shevchenko wrote:
>>> On Thu, Jun 8, 2017 at 6:05 PM, Alexandre Belloni
>>> <alexandre.belloni@free-electrons.com> wrote:
>
>>> > I understand this may not fit your debugging needs but what about pretty
>>> > printing time64_t and using rtc_tm_to_time64?
>>>
>>> There are two downsides as I can see:
>>> 1) conversion to and from just for that;
>>
>> Those are almost all debug messages, I would be fine with that.
>
> Yeah, but the problem is to pass the reference. All dances around will
> uglify the code.
> (Obviously we can't pass timespec64/time64_t or anything longer than
> 32 bits as is in %p extension)
>

I like that this gets rid of some mm/dd/yy and other more or less random
format and ends up standardizing yyyy-mm-dd HH:MM:SS. However, I do
think %pt should take either ktime_t or timespec64 (obviously by
reference), with fx these options

[ir] ISO (yyyy-mm-dd HH:MM:SS) or raw (seconds since epoch)
[n] append nanoseconds (.%09ld).

Please don't give people the option of eliding either the time or the
date; I've spent too much time dealing with syslog files that don't
include the year in the timestamps.

Getting a timespec64* or ktime_t* from <something else> is not that
bad. There's the compound literal option

#define rtc_tm2timespec64p(tm) \
 (&(struct timespec64){ .tv_sec = rtc_tm_to_time64(tm), .tv_nsec = 0 })

printk("%pt", rtc_tm2timespec64p(tm))

or the two-extra-lines per call-site

struct timespec64 ts;
rtc_tm2time64(tm, &ts)
printk("%pt", &ts)


Rasmus
Andy Shevchenko June 8, 2017, 9:25 p.m. UTC | #11
On Thu, Jun 8, 2017 at 11:42 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Thu, Jun 08 2017, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> On Thu, Jun 8, 2017 at 9:41 PM, Alexandre Belloni
>> <alexandre.belloni@free-electrons.com> wrote:
>>> On 08/06/2017 at 20:57:05 +0300, Andy Shevchenko wrote:
>>>> On Thu, Jun 8, 2017 at 6:05 PM, Alexandre Belloni
>>>> <alexandre.belloni@free-electrons.com> wrote:

>> Yeah, but the problem is to pass the reference. All dances around will
>> uglify the code.
>> (Obviously we can't pass timespec64/time64_t or anything longer than
>> 32 bits as is in %p extension)

> I like that this gets rid of some mm/dd/yy and other more or less random
> format and ends up standardizing yyyy-mm-dd HH:MM:SS. However, I do
> think %pt should take either ktime_t or timespec64 (obviously by
> reference),

I will try to look in this direction.

> with fx these options
>
> [ir] ISO (yyyy-mm-dd HH:MM:SS) or raw (seconds since epoch)
> [n] append nanoseconds (.%09ld).

We still need to be able to print time *and/or* year groups
separately. There are users which provide that via procfs or sysfs
(ABI).

> Please don't give people the option of eliding either the time or the
> date; I've spent too much time dealing with syslog files that don't
> include the year in the timestamps.

I understand that, but see above.

> Getting a timespec64* or ktime_t* from <something else> is not that
> bad. There's the compound literal option

Will look at it later, thanks for it.

>
> #define rtc_tm2timespec64p(tm) \
>  (&(struct timespec64){ .tv_sec = rtc_tm_to_time64(tm), .tv_nsec = 0 })
>
> printk("%pt", rtc_tm2timespec64p(tm))
>
> or the two-extra-lines per call-site
>
> struct timespec64 ts;
> rtc_tm2time64(tm, &ts)
> printk("%pt", &ts)
Arnd Bergmann June 8, 2017, 9:45 p.m. UTC | #12
On Thu, Jun 8, 2017 at 11:25 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Jun 8, 2017 at 11:42 PM, Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>> On Thu, Jun 08 2017, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Thu, Jun 8, 2017 at 9:41 PM, Alexandre Belloni
>>> <alexandre.belloni@free-electrons.com> wrote:
>>>> On 08/06/2017 at 20:57:05 +0300, Andy Shevchenko wrote:
>>>>> On Thu, Jun 8, 2017 at 6:05 PM, Alexandre Belloni
>>>>> <alexandre.belloni@free-electrons.com> wrote:
>
>>> Yeah, but the problem is to pass the reference. All dances around will
>>> uglify the code.
>>> (Obviously we can't pass timespec64/time64_t or anything longer than
>>> 32 bits as is in %p extension)
>
>> I like that this gets rid of some mm/dd/yy and other more or less random
>> format and ends up standardizing yyyy-mm-dd HH:MM:SS. However, I do
>> think %pt should take either ktime_t or timespec64 (obviously by
>> reference),
>
> I will try to look in this direction.

sounds good.

>> Please don't give people the option of eliding either the time or the
>> date; I've spent too much time dealing with syslog files that don't
>> include the year in the timestamps.
>
> I understand that, but see above.

When we pretty-print a ktime_t, we probably want to leave out the high
fields as well, as this often refers to a time interval, e.g. a few seconds.
Even for absolute values, the start of ktime_t is usually not the 1970
epoch but system boot, so we may not necessarily want the higher
fields. I hoped to find some inspiration in the 'date' man page, which
contains a lot of formatting options, but it's hard to translate that into
a useful format string within the constraints of %p flags in printk.

     Arnd
Joe Perches June 8, 2017, 11:09 p.m. UTC | #13
On Thu, 2017-06-08 at 21:02 +0300, Andy Shevchenko wrote:
> On Thu, Jun 8, 2017 at 6:33 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Jun 8, 2017 at 4:55 PM, Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Thu, Jun 8, 2017 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Thu, Jun 8, 2017 at 3:47 PM, Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > I really like the idea, and the implementation seems fine for this use case, but
> > > > before we reserve %pt for rtc_time, could we discuss whether we want
> > > > that for printing struct tm, struct timespec64, time64_t or ktime_t instead?
> > > 
> > > How many users?
> > 
> > It's hard to predict, I would assume we get more users once there is an
> > easy way to print the time.
> 
> So, at least for now we can guess using existing users, right?
> 
> I don't check yet how to calculate those cases of time64_t,
> timespec64, ktime_t and alike if they are about pretty ptintong time
> and date.
> I'm speculating that there are (almost) none.
> 
> > > For struct tm it's somelike 4 (which want to print its content).
> > 
> > Good point. I notice that they all convert from time64_t or time_t into
> > struct tm immediately before printing it, so we can scratch that one
> > as long as there is a way to pretty-print a time64_t. We also don't
> > need to print a time_t as we want to kill that one off anyway.
> > 
> > If we only care about printing time64_t and rtc_time, we can easily
> > use %pT for one and %pt for the other, but there may still be good
> > reasons to print a timespec64 or ktime_t.
> 
> No need, we may still use 3rd/4th letter in the format for that.
> 
> %pt(t/d) time/date + whatever modifications, like raw, validate, timespec, etc.
> 
> 's' for timespec64, for example.

My preference would be for %pt[type]<output style>
where <type> is mandatory and could be:

	r for struct rtc_time
	6 for time64_t
	k for ktime_t
	T for struct timespec64
	etc

and <output style> has an unspecified default of
YYYY-MM-DD:hh:mm:ss

Perhaps use the "date" formats without the leading
% uses for <output style> for additional styles.
Andy Shevchenko July 20, 2017, 10:30 a.m. UTC | #14
On Thu, 2017-06-08 at 23:45 +0200, Arnd Bergmann wrote:
> On Thu, Jun 8, 2017 at 11:25 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jun 8, 2017 at 11:42 PM, Rasmus Villemoes
> > <linux@rasmusvillemoes.dk> wrote:
> > > On Thu, Jun 08 2017, Andy Shevchenko <andy.shevchenko@gmail.com>
> > > wrote:
> > > > On Thu, Jun 8, 2017 at 9:41 PM, Alexandre Belloni
> > > > <alexandre.belloni@free-electrons.com> wrote:
> > > > > On 08/06/2017 at 20:57:05 +0300, Andy Shevchenko wrote:
> > > > > > On Thu, Jun 8, 2017 at 6:05 PM, Alexandre Belloni
> > > > > > <alexandre.belloni@free-electrons.com> wrote:
> > > > Yeah, but the problem is to pass the reference. All dances
> > > > around will
> > > > uglify the code.
> > > > (Obviously we can't pass timespec64/time64_t or anything longer
> > > > than
> > > > 32 bits as is in %p extension)
> > > I like that this gets rid of some mm/dd/yy and other more or less
> > > random
> > > format and ends up standardizing yyyy-mm-dd HH:MM:SS. However, I
> > > do
> > > think %pt should take either ktime_t or timespec64 (obviously by
> > > reference),
> > 
> > I will try to look in this direction.
> 
> sounds good.
> 
> > > Please don't give people the option of eliding either the time or
> > > the
> > > date; I've spent too much time dealing with syslog files that
> > > don't
> > > include the year in the timestamps.
> > 
> > I understand that, but see above.
> 
> When we pretty-print a ktime_t, we probably want to leave out the high
> fields as well, as this often refers to a time interval, e.g. a few
> seconds.
> Even for absolute values, the start of ktime_t is usually not the 1970
> epoch but system boot, so we may not necessarily want the higher
> fields. I hoped to find some inspiration in the 'date' man page, which
> contains a lot of formatting options, but it's hard to translate that
> into
> a useful format string within the constraints of %p flags in printk.

Rasmus et al.,

Summarizing this discussion I would go forward with the following

- add one more letter in the format to provide argument type (timespec,
ktime, ...)

- make a config option to enable / disable this facility and select it
by users (and/or make it visible for configuration?)

- still leave possibility to print either date or time or both

- add suffix to print nanoseconds in cases where input has them (and
output is not just plain date)

- address other (technical) comments
diff mbox

Patch

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index d8c40c30118a..fa5b718a5dcc 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -282,6 +282,23 @@  struct va_format:
 
 	Passed by reference.
 
+struct rtc_time:
+
+	%pt		YYYY-mm-dd HH:MM:SS
+	%ptd		YYYY-mm-dd
+	%ptt		HH:MM:SS
+	%pt[dt][rv]
+
+	For printing date and time as represented by struct rtc_time
+	structure in human readable format.
+
+	By default year will be incremented by 1900 and month by 1. Use
+	'r' (raw) to suppress this behaviour. On the other hand when
+	'v' is applied validation mechanism will be in use, i.e. numbers
+	out of range will be replaced by '**' or '****'.
+
+	Passed by reference.
+
 struct clk:
 
 	%pC	pll1
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 001303612b55..653d08b50850 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -30,6 +30,7 @@ 
 #include <linux/ioport.h>
 #include <linux/dcache.h>
 #include <linux/cred.h>
+#include <linux/rtc.h>
 #include <linux/uuid.h>
 #include <net/addrconf.h>
 #ifdef CONFIG_BLOCK
@@ -701,6 +702,20 @@  static const struct printf_spec default_dec_spec = {
 	.precision = -1,
 };
 
+static const struct printf_spec default_dec02_spec = {
+	.base = 10,
+	.field_width = 2,
+	.precision = -1,
+	.flags = ZEROPAD,
+};
+
+static const struct printf_spec default_dec04_spec = {
+	.base = 10,
+	.field_width = 4,
+	.precision = -1,
+	.flags = ZEROPAD,
+};
+
 static noinline_for_stack
 char *resource_string(char *buf, char *end, struct resource *res,
 		      struct printf_spec spec, const char *fmt)
@@ -1382,6 +1397,112 @@  char *address_val(char *buf, char *end, const void *addr, const char *fmt)
 }
 
 static noinline_for_stack
+char *date_string(char *buf, char *end, const struct rtc_time *tm, bool v, bool r)
+{
+	int year = tm->tm_year + (r ? 0 : 1900);
+	int mon = tm->tm_mon + (r ? 0 : 1);
+
+	if (unlikely(v && (unsigned int)tm->tm_year > 200))
+		buf = string(buf, end, "****", default_str_spec);
+	else
+		buf = number(buf, end, year, default_dec04_spec);
+
+	if (buf < end)
+		*buf = '-';
+	buf++;
+
+	if (unlikely(v && (unsigned int)tm->tm_mon > 11))
+		buf = string(buf, end, "**", default_str_spec);
+	else
+		buf = number(buf, end, mon, default_dec02_spec);
+
+	if (buf < end)
+		*buf = '-';
+	buf++;
+
+	if (unlikely(v && (unsigned int)tm->tm_mday > 31))
+		buf = string(buf, end, "**", default_str_spec);
+	else
+		buf = number(buf, end, tm->tm_mday, default_dec02_spec);
+
+	return buf;
+}
+
+static noinline_for_stack
+char *time_string(char *buf, char *end, const struct rtc_time *tm, bool v, bool r)
+{
+	if (unlikely(v && (unsigned int)tm->tm_hour > 24))
+		buf = string(buf, end, "**", default_str_spec);
+	else
+		buf = number(buf, end, tm->tm_hour, default_dec02_spec);
+
+	if (buf < end)
+		*buf = ':';
+	buf++;
+
+	if (unlikely(v && (unsigned int)tm->tm_min > 59))
+		buf = string(buf, end, "**", default_str_spec);
+	else
+		buf = number(buf, end, tm->tm_min, default_dec02_spec);
+
+	if (buf < end)
+		*buf = ':';
+	buf++;
+
+	if (unlikely(v && (unsigned int)tm->tm_sec > 59))
+		buf = string(buf, end, "**", default_str_spec);
+	else
+		buf = number(buf, end, tm->tm_sec, default_dec02_spec);
+
+	return buf;
+}
+
+static noinline_for_stack
+char *timeanddate(char *buf, char *end, const struct rtc_time *tm, const char *fmt)
+{
+	bool have_t = true, have_d = true;
+	bool validate = false;
+	bool raw = false;
+	int count = 0;
+	bool found;
+
+	switch (fmt[++count]) {
+	case 'd':
+		have_t = false;
+		break;
+	case 't':
+		have_d = false;
+		break;
+	}
+
+	/* No %pt[dt] supplied */
+	if (have_t && have_d)
+		--count;
+
+	found = true;
+	do {
+		switch (fmt[++count]) {
+		case 'r':
+			raw = true;
+			break;
+		case 'v':
+			validate = true;
+			break;
+		default:
+			found = false;
+			break;
+		}
+	} while (found);
+
+	if (have_d)
+		buf = date_string(buf, end, tm, validate, raw);
+	if (have_t)
+		buf = time_string(buf, end, tm, validate, raw);
+
+	return buf;
+}
+
+static noinline_for_stack
 char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
 	    const char *fmt)
 {
@@ -1550,6 +1671,7 @@  int kptr_restrict __read_mostly;
  * - 'd[234]' For a dentry name (optionally 2-4 last components)
  * - 'D[234]' Same as 'd' but for a struct file
  * - 'g' For block_device name (gendisk + partition number)
+ * - 't[dt][rv]' For time and date as represented in struct rtc_time
  * - 'C' For a clock, it prints the name (Common Clock Framework) or address
  *       (legacy clock framework) of the clock
  * - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
@@ -1702,6 +1824,8 @@  char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		return address_val(buf, end, ptr, fmt);
 	case 'd':
 		return dentry_name(buf, end, ptr, spec, fmt);
+	case 't':
+		return timeanddate(buf, end, (const struct rtc_time *)ptr, fmt);
 	case 'C':
 		return clock(buf, end, ptr, spec, fmt);
 	case 'D':