-----Original Message-----
> 在 2020年09月11日 14:01, HAGIO KAZUHITO(萩尾 一仁) 写道:
> > 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?
> >
> No problem, this is also acceptable because it is not a serious problem.
>
> Thanks.
> Acked-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
Thank you for understanding, applied.
Kazu
>
> > 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
> >>>
> >
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
