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

[lijiang]

在 2020年08月21日 09:49, 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>
> ---
> v1 -> v2
> - Add comment about the fallbacks in ctime_tz()
> - Add NULL check on timep
> 
>  defs.h   |  1 +
>  help.c   |  3 +--
>  kernel.c | 16 ++++++----------
>  tools.c  | 32 ++++++++++++++++++++++++++++++++
>  4 files changed, 40 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..89352b1dc5f5 100644
> --- a/tools.c
> +++ b/tools.c
> @@ -6318,6 +6318,38 @@ 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').  If localtime() or strftime() fails, fail back to return
> + * POSIX time (seconds since the Epoch) or ctime() string respectively.
> + *
> + * NOTE: The return value points to a statically allocated string which is
> + * overwritten by subsequent calls.
> + */

Although it simplifies the context in which it is called, the definition of the
static variable could not be very good, and that could leave some potential risks
as you mentioned in the code comment.

In addition, I noticed that there are not many calls to the ctime_tz(). Could it
be better like this? 

int ctime_tz(time_t *timep, char *buf, int size)

...
ret = ctime_tz(&time_now, buffer, sizeof(buffer));
...

Thanks.
Lianbo

> +char *
> +ctime_tz(time_t *timep)
> +{
> +	static char buf[64];
> +	struct tm *tm;
> +	size_t size;
> +
> +	if (!timep)
> +		return NULL;
> +
> +	tm = localtime(timep);
> +	if (!tm) {
> +		snprintf(buf, sizeof(buf), "%ld", *timep);
> +		return buf;
> +	}
> +
> +	size = strftime(buf, sizeof(buf), "%a %b %e %T %Z %Y", tm);
> +	if (!size)
> +		return strip_linefeeds(ctime(timep));
> +
> +	return buf;
> +}
> +
> +/*
>   *  Stall for a number of microseconds.
>   */
>  void
>