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

Hi Lianbo, Thank you for reviewing this. -----Original Message----- ... >> /* >> + * 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)); > ... Yeah it might be safer, but I would not like to have an additional buffer only for this functionality and there is no cases that the allocated buffer is reused in the current crash source. That behavior and interface are same as the original ctime() (please see the manpage of ctime), so we can just replace it with the ctime_tz() and it will be convenient for developers who are used to ctime(). I took the convenience rather than the safety here. Is it unacceptable?

Thanks, Kazu > > 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 >>