[Crash-utility] [PATCH] Append time zone to output of date and time
HAGIO KAZUHITO(萩尾 一仁)
k-hagio-ab at nec.com
Thu Aug 20 01:54:04 UTC 2020
Hi Mathias,
Thanks for the review!
-----Original Message-----
> 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.
but if tm is NULL, then the fallback ctime() also will return NULL
because ctime(t) is equivalent to asctime(localtime(t)).
Aren't "UNIX seconds since epoch" better than "(null)" ?
>
> > +
> > + 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?
Agreed, will do in v2.
Thanks,
Kazu
>
> > +
> > + 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
More information about the Crash-utility
mailing list