Re: [PATCH v2 01/21] lib/vsprintf: Print time and date in human readable format via %pt

From: Geert Uytterhoeven
Date: Wed Feb 21 2018 - 04:33:58 EST


Hi Andy,

On Tue, Feb 20, 2018 at 10:43 PM, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> 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 %ptR[dt][rv] specifier.

Thanks for your patch!

> Note, users have to select PRINTK_PEXT_TIMEDATE option in a Kconfig.

Is it worthwhile making this an option?

> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -412,6 +412,37 @@ Examples::
>
> Passed by reference.
>
> +Time and date
> +-------------
> +
> +::
> +
> + %pt[R] YYYY-mm-dd HH:MM:SS
> + %pt[R]d YYYY-mm-dd
> + %pt[R]t HH:MM:SS

[R] suggests the "R" is optional?
But if it's missing, it prints the hex pointer value?

> + %pt[R][dt]

What's the purpose of this?

> +
> + R for struct rtc_time
> +
> +Note, users have to select PRINTK_PEXT_TIMEDATE option in a Kconfig.
> +
> +struct rtc_time
> +~~~~~~~~~~~~~~~
> +
> +::
> +
> + %ptR[dt][rv]

What's the purpose of this paragraph, compared to the previous one?

> +
> +For printing date and time as represented by struct rtc_time structure in
> +human readable format.

> @@ -1443,6 +1458,132 @@ char *address_val(char *buf, char *end, const void *addr, const char *fmt)
> return special_hex_number(buf, end, num, size);
> }
>
> +static noinline_for_stack
> +char *date_str(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 = '-';

Instead of all these checks to avoid overflowing the passed buffer, it
may be simpler to format everything in a fixed-size buffer on the stack,
and copy whatever will fit in the target buffer at the end.

> + 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_str(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 = ':';

Likewise.

> + 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 *rtc_str(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 = 1;
> + bool found;
> +
> + switch (fmt[++count]) {
> + case 'd':
> + have_t = false;
> + break;
> + case 't':
> + have_d = false;
> + break;
> + }
> +
> + /* No %pt[dt] supplied */
> + if (have_d && have_t)
> + --count;

First increment count, then rollback.
What about:

switch (fmt[count]) {
case 'd':
have_t = false;
count++;
break;
case 't':
have_d = false;
count++;
break;
}

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds