linux-kernel - Re: [PATCH v5 02/21] lib/vsprintf: Print time and date in human readable format via %pt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181204133028.xkdnc5bqtqlnb3zu@pathway.suse.cz>
Date:   Tue, 4 Dec 2018 14:30:28 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        linux-rtc@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
        Joe Perches <joe@...ches.com>,
        Mark Salyzyn <salyzyn@...roid.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        linux-kernel@...r.kernel.org,
        Rasmus Villemoes <rasmus.villemoes@...vas.dk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        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 v5 02/21] lib/vsprintf: Print time and date in human
 readable format via %pt

On Thu 2018-11-29 12:59:40, Andy Shevchenko 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][r] specifier.
> 
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 53527ea822b5..97b7d14961d6 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -418,6 +418,11 @@ struct_va_format(void)
>  {
>  }
>  
> +static void __init
> +struct_rtc_time(void)
> +{
> +}

Just by chance, do you have any plans to add the test code? ;-)

I understand that you did now want to spend time on it before
the real change was accepted.


>  static void __init
>  struct_clk(void)
>  {
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 37a54a6dd594..b7114799b91f 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> +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 raw = false;
> +	int count = 2;
> +	bool found;
> +
> +	switch (fmt[count]) {
> +	case 'd':
> +		have_t = false;
> +		count++;
> +		break;
> +	case 't':
> +		have_d = false;
> +		count++;
> +		break;
> +	}
> +
> +	found = true;
> +	do {
> +		switch (fmt[count++]) {
> +		case 'r':
> +			raw = true;
> +			break;
> +		default:
> +			found = false;
> +			break;
> +		}
> +	} while (found);

I guess that the while cycle is remainder from an older version and
should not be here. It handles only the final 'r' now. 


> +	if (have_d)
> +		buf = date_str(buf, end, tm, raw);
> +	if (have_d && have_t) {
> +		/* Respect ISO 8601 */
> +		if (buf < end)
> +			*buf = 'T';

I checked several conversion patches and the original code did not use
the ISO format. The change makes sense (even though I personally
do not like the format much ;-)

Anyway, people might expect that the conversion is 1:1. The change
should get mentioned in the affected patches so that people are
not later surprised.


> +		buf++;
> +	}
> +	if (have_t)
> +		buf = time_str(buf, end, tm, raw);
> +
> +	return buf;
> +}
> +
> +static noinline_for_stack
> +char *timeanddate(char *buf, char *end, void *ptr, struct printf_spec spec,
> +		  const char *fmt)

Please, rename the function to time_and_date(). It is the style used
in this source file and it is much easier to read.

Otherwise, the patch looks fine. It helps to remove the many variants
of the code and unifies the output format.

Best Regards,
Petr