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

HAGIO KAZUHITO(萩尾 一仁) k-hagio-ab at nec.com
Mon Sep 14 07:51:49 UTC 2020


-----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 at redhat.com>

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
> >>>
> >





More information about the Crash-utility mailing list