linux-kernel - Re: [PATCH v1 1/4] lib/vsprintf: Allow to override date and time separator

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: <YJqRYNlAu0SfWpb5@alley>
Date:   Tue, 11 May 2021 16:14:56 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     JC Kuo <jckuo@...dia.com>, Joe Perches <joe@...ches.com>,
        Sumit Garg <sumit.garg@...aro.org>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-nilfs@...r.kernel.org,
        kgdb-bugreport@...ts.sourceforge.net,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Jonathan Corbet <corbet@....net>,
        Mathias Nyman <mathias.nyman@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Ryusuke Konishi <konishi.ryusuke@...il.com>,
        Jason Wessel <jason.wessel@...driver.com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH v1 1/4] lib/vsprintf: Allow to override date and time
 separator

On Mon 2021-05-10 18:04:10, Andy Shevchenko wrote:
> ISO 8601 defines 'T' as a separator between date and time. Though,
> some ABIs use time and date with ' ' separator instead.
> 
> Add a flavour to the %pt specifier to override default separator.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
>  Documentation/core-api/printk-formats.rst |  6 +++++-
>  lib/test_printf.c                         |  5 +++++
>  lib/vsprintf.c                            | 19 ++++++++++++++++---
>  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index f063a384c7c8..bc85fd4685e7 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -514,9 +514,10 @@ Time and date
>  ::
>  
>  	%pt[RT]			YYYY-mm-ddTHH:MM:SS
> +	%pt[RT]s		YYYY-mm-dd HH:MM:SS
>  	%pt[RT]d		YYYY-mm-dd
>  	%pt[RT]t		HH:MM:SS
> -	%pt[RT][dt][r]
> +	%pt[RT][dt][rs]

Sigh, we do not have clear rules what [xy] means. It might be:

      + always use one of them
      + optionally use one of them
      + always use any of them
      + optionally use any of them

%pt[RT][dt][rs] is a great mix:

      + R or T is required, the rest is optional
      + 'd' or 't' can be used but both together are not supported 
      + any variant of 'r' and 's' is supported including various ordering

Honestly, I do not know about any magic solution that might make it
easier to understand these monster modifiers.

Well, what about using the following at least in this case:

	%pt[RT][dt][r][s]

It might help to understand that both 'r' and 's' can be used at the
same time.

An attempt to distinguishing all the possibilities might be:

	%pt{RT}[{dt}][r][s]

where [] means that it is optional and {} means one of them must be
chosen. But I am not sure if it really makes the life easier. Anyway,
this would be for another patch that updates the entire printk-formats.rst.

>  
>  For printing date and time as represented by::
>  
> @@ -528,6 +529,9 @@ in human readable format.
>  By default year will be incremented by 1900 and month by 1.
>  Use %pt[RT]r (raw) to suppress this behaviour.
>  
> +The %pt[RT]s (space) will override ISO 8601 by using ' ' instead of 'T'
> +between date and time. It won't have any effect when date or time is omitted.
> +
>  Passed by reference.
>  
>  struct clk
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index f0c35d9b65bf..5f36c7a43cdc 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1834,7 +1834,8 @@ char *rtc_str(char *buf, char *end, const struct rtc_time *tm,
>  	      struct printf_spec spec, const char *fmt)
>  {
>  	bool have_t = true, have_d = true;
> -	bool raw = false;
> +	bool raw = false, space = false;
> +	bool found = true;
>  	int count = 2;
>  
>  	if (check_pointer(&buf, end, tm, spec))
> @@ -1851,14 +1852,26 @@ char *rtc_str(char *buf, char *end, const struct rtc_time *tm,
>  		break;
>  	}
>  
> -	raw = fmt[count] == 'r';
> +	do {
> +		switch (fmt[count++]) {
> +		case 'r':
> +			raw = true;
> +			break;
> +		case 's':
> +			space = true;
> +			break;
> +		default:
> +			found = false;
> +			break;
> +		}
> +	} while (found);
>  
>  	if (have_d)
>  		buf = date_str(buf, end, tm, raw);
>  	if (have_d && have_t) {
>  		/* Respect ISO 8601 */

The comment is slightly misleding now. What about something like?

		/* 'T' by ISO 8601. */

Or maybe call the variable: iso_8601, remove the comment, and
invert the logic:


		bool iso_8601 = true;

		case 's':
			iso_8601 = false;
			break;

		*buf = iso_8601 ? 'T' : ' ';

>  		if (buf < end)
> -			*buf = 'T';
> +			*buf = space ? ' ' : 'T';
>  		buf++;
>  	}
>  	if (have_t)

Best Regards,
Petr