Am 20.08.20 um 03:54 schrieb HAGIO KAZUHITO(萩尾 一仁): > 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@xxxxxxxxxx> >>> Signed-off-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx> >>> --- >>> 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)" ? Valid point! Agreed. >>> + >>> + 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, Mathias > > 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 -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
