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....@oracle.com>
> Signed-off-by: Kazuhito Hagio <k-hagio...@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
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
