在 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 <liji...@redhat.com>
> 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@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
