[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdVwjP6cWM=Ku6KRVz9F8_n4OsgeK42UZa6BenKij4iu4Q@mail.gmail.com>
Date: Wed, 21 Feb 2018 10:33:51 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Rasmus Villemoes <rasmus.villemoes@...vas.dk>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Alessandro Zummo <a.zummo@...ertech.it>,
Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
linux-rtc@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
Joe Perches <joe@...ches.com>,
Mark Salyzyn <salyzyn@...roid.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Guan Xuetao <gxt@...c.pku.edu.cn>,
Ingo Molnar <mingo@...nel.org>,
Jason Wessel <jason.wessel@...driver.com>,
Jonathan Corbet <corbet@....net>,
Jonathan Hunter <jonathanh@...dia.com>,
Krzysztof Kozlowski <krzk@...nel.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Thierry Reding <thierry.reding@...il.com>
Subject: Re: [PATCH v2 01/21] lib/vsprintf: Print time and date in human
readable format via %pt
Hi Andy,
On Tue, Feb 20, 2018 at 10:43 PM, Andy Shevchenko
<andriy.shevchenko@...ux.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 %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@...ux-m68k.org
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
Powered by blists - more mailing lists