Q: Can the current asctime_r() implementation potentially be threat unsafe?

[Jeff Johnston]

Christian Baribeau wrote:
> Allo!
> 
> I may have found a potential bug in the reentrant implementation of the
> asctime_r(). Here is the implementation taken from newlib CVS.
> 
> char *
> _DEFUN (asctime_r, (tim_p, result),
> _CONST struct tm *tim_p _AND
> char *result)
> {
> static _CONST char day_name[7][3] = {
> "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
> };
> static _CONST char mon_name[12][3] = {
> "Jan", "Feb", "Mar", "Apr", "May", "Jun",
> "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
> };
> 
> sprintf (result, "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n", <- Potential Problem
> day_name[tim_p->tm_wday],
> mon_name[tim_p->tm_mon],
> tim_p->tm_mday, tim_p->tm_hour, tim_p->tm_min,
> tim_p->tm_sec, 1900 + tim_p->tm_year);
> return result;
> }
> 
> Can someone confirm that the call to sprintf() should be replaced with
> _sprintf_r() with the appropriate struct _reent in order to make asctime_r()
> reentrant or that the current implementation can be considered "thread
> safe".
> 
> Thank you,
> 
> Christian
>

It is fine Christian.  With sprintf, the file struct passed to _vfprintf_r is on 
the stack so any file locking is irrelevant.  The only reentrancy call made in 
_vfprintf_r will be to _mbtowc_r and it is called with a state field that is a 
local variable.  Even though the errno field to reference will not be the 
thread's errno field, the format string is single-byte so errno will not be 
modified by this call.  There are no other reentrancy struct items that can be 
modified (e.g. no other ways to cause errno to be set) so the call is fine.

Note that asctime_r is not a newlib invention like other _r functions.  It does 
not take a reentrancy struct argument.

-- Jeff J.