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

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(a)oracle.com&gt; > Signed-off-by: Kazuhito Hagio <k-hagio-ab(a)nec.com&gt; > --- > 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