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

HAGIO KAZUHITO(萩尾 一仁) k-hagio-ab at nec.com
Fri Sep 11 06:01:09 UTC 2020


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





More information about the Crash-utility mailing list