[Crash-utility] [PATCH] Append time zone to output of date and time

[Mathias Krause]

Hi Kazuhito,

Am 19.08.20 um 08:32 schrieb HAGIO KAZUHITO(萩尾 一仁):
> Currently it's not easy to distinguish which time zone the output of
> DATE uses:
> 
>   crash> sys | grep DATE
>         DATE: Thu Nov 29 06:44:02 2018
> 
> Let's introduce ctime_tz() function like ctime() but explicitly appends
> the time zone to its result string.
> 
>         DATE: Thu Nov 29 06:44:02 JST 2018
> 
> Resolves: https://github.com/crash-utility/crash/issues/62
> Suggested-by: Jacob Wen <jian.w.wen at oracle.com>
> Signed-off-by: Kazuhito Hagio <k-hagio-ab at nec.com>
> ---
>  defs.h   |  1 +
>  help.c   |  3 +--
>  kernel.c | 16 ++++++----------
>  tools.c  | 28 ++++++++++++++++++++++++++++
>  4 files changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index 17e98763362b..e7dec7c672a6 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -5096,6 +5096,7 @@ char *strdupbuf(char *);
>  void sigsetup(int, void *, struct sigaction *, struct sigaction *);
>  #define SIGACTION(s, h, a, o) sigsetup(s, h, a, o)
>  char *convert_time(ulonglong, char *);
> +char *ctime_tz(time_t *);
>  void stall(ulong);
>  char *pages_to_size(ulong, char *);
>  int clean_arg(void);
> diff --git a/help.c b/help.c
> index b707cfa58826..d3427a36829f 100644
> --- a/help.c
> +++ b/help.c
> @@ -9299,8 +9299,7 @@ display_README(void)
>  			fprintf(fp, "    GNU gdb %s\n", pc->gdb_version);
>  		} else if (STREQ(README[i], README_DATE)) {
>      			time(&time_now);               
> -        		fprintf(fp, "            DATE: %s\n", 
> -                		strip_linefeeds(ctime(&time_now)));
> +			fprintf(fp, "            DATE: %s\n", ctime_tz(&time_now));
>  		} else if (STREQ(README[i], README_HELP_MENU)) {
>  			display_help_screen("    ");
>  		} else if (STREQ(README[i], README_GPL_INFO)) {
> diff --git a/kernel.c b/kernel.c
> index f179375f2d3d..92bfe6329900 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -225,10 +225,9 @@ kernel_init()
>  	get_xtime(&kt->date);
>  	if (CRASHDEBUG(1))
>  		fprintf(fp, "xtime timespec.tv_sec: %lx: %s\n", 
> -			kt->date.tv_sec, strip_linefeeds(ctime(&kt->date.tv_sec)));
> +			kt->date.tv_sec, ctime_tz(&kt->date.tv_sec));
>  	if (kt->flags2 & GET_TIMESTAMP) {
> -		fprintf(fp, "%s\n\n", 
> -			strip_linefeeds(ctime(&kt->date.tv_sec)));
> +		fprintf(fp, "%s\n\n", ctime_tz(&kt->date.tv_sec));
>  		clean_exit(0);
>  	}
>  	
> @@ -5178,7 +5177,7 @@ dump_log_entry(char *logptr, int msg_flags)
>  		rem = (ulonglong)ts_nsec % (ulonglong)1000000000;
>  		if (msg_flags & SHOW_LOG_CTIME) {
>  			time_t t = kt->boot_date.tv_sec + nanos;
> -			sprintf(buf, "[%s] ", strip_linefeeds(ctime(&t)));
> +			sprintf(buf, "[%s] ", ctime_tz(&t));
>  		}
>  		else
>  			sprintf(buf, "[%5lld.%06ld] ", nanos, rem/1000);
> @@ -5553,8 +5552,7 @@ display_sys_stats(void)
>  
>  	if (ACTIVE())
>  		get_xtime(&kt->date);
> -        fprintf(fp, "        DATE: %s\n", 
> -		strip_linefeeds(ctime(&kt->date.tv_sec))); 
> +        fprintf(fp, "        DATE: %s\n", ctime_tz(&kt->date.tv_sec));
>          fprintf(fp, "      UPTIME: %s\n", get_uptime(buf, NULL)); 
>          fprintf(fp, "LOAD AVERAGE: %s\n", get_loadavg(buf)); 
>  	fprintf(fp, "       TASKS: %ld\n", RUNNING_TASKS());
> @@ -6043,10 +6041,8 @@ dump_kernel_table(int verbose)
>  		kt->source_tree : "(not used)");
>  	if (!(pc->flags & KERNEL_DEBUG_QUERY) && ACTIVE()) 
>  		get_xtime(&kt->date);
> -        fprintf(fp, "          date: %s\n",
> -                strip_linefeeds(ctime(&kt->date.tv_sec)));
> -        fprintf(fp, "    boot_ date: %s\n",
> -                strip_linefeeds(ctime(&kt->boot_date.tv_sec)));
> +        fprintf(fp, "          date: %s\n", ctime_tz(&kt->date.tv_sec));
> +        fprintf(fp, "     boot_date: %s\n", ctime_tz(&kt->boot_date.tv_sec));
>          fprintf(fp, "  proc_version: %s\n", strip_linefeeds(kt->proc_version));
>          fprintf(fp, "   new_utsname: \n");
>          fprintf(fp, "      .sysname: %s\n", uts->sysname);
> diff --git a/tools.c b/tools.c
> index 85c81668e40e..4654e7458a3e 100644
> --- a/tools.c
> +++ b/tools.c
> @@ -6318,6 +6318,34 @@ convert_time(ulonglong count, char *buf)
>  }
>  
>  /*
> + * Convert a calendar time into a null-terminated string like ctime(), but
> + * the result string contains the time zone string and does not ends with a
> + * linefeed ('\n').
> + *
> + * NOTE: The return value points to a statically allocated string which is
> + * overwritten by subsequent calls.
> + */
> +char *
> +ctime_tz(time_t *timep)
> +{
> +	static char buf[64];
> +	struct tm *tm;
> +	size_t size;
> +
> +	tm = localtime(timep);

> +	if (!tm) {
> +		snprintf(buf, sizeof(buf), "%ld", *timep);
> +		return buf;
> +	}

I don't think this is especially useful, better fall back to ctime() in
this case, but see below.

> +
> +	size = strftime(buf, sizeof(buf), "%a %b %e %T %Z %Y", tm);
> +	if (!size)
> +		return strip_linefeeds(ctime(timep));

Maybe you should mention in the leading comment that this function falls
back to ctime() in case it fails to generate a timestamp with the time
zone info?

> +
> +	return buf;
> +}
> +
> +/*
>   *  Stall for a number of microseconds.
>   */
>  void

So, to simplify ctime_tz() and avoid the "UNIX seconds since epoch" case
I'd suggest something like this instead:

  char *
  ctime_tz(time_t *timep)
  {
  	static char buf[64];
  	struct tm *tm;

  	tm = localtime(timep);
  	if (tm && strftime(buf, sizeof(buf), "%a %b %e %T %Z %Y", tm))
  		return buf;

  	return strip_linefeeds(ctime(timep));
  }

Beside that, patch looks good to me!

Thanks,
Mathias