Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> Cc: bug-gnu-emacs@gnu.org
> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Date: Mon, 27 Mar 2006 14:00:15 -0800
> 
> Here's your earlier explanation:
> 
>    I wouldn't remove these: the functions are almost trivial wrappers
>    around the library version, and someone could try using ctime in the
>    future (in a different context), in which case they will hit the
>    Windows library bug again.
> 
> But this explanation is problematic, at least in the context of an
> Emacs that is intended to be portable to 64-bit hosts.  Here's why.
> The ntlib.c and w32 wrappers do this:
> 
>   char *str = (char *) ctime (t);
>   return (str ? str : "Sun Jan 01 00:00:00 1970");
> 
> That is, the wrappers cause 'ctime' to always return a non-NULL pointer.
> But any future Emacs code that uses ctime on an arbitrary time stamp
> cannot expect ctime to always return a non-null pointer.  Even glibc
> ctime (a "nice" ctime, which always has well-defined behavior) returns
> NULL in some cases, e.g., if given a time stamp equal to 2**56.

The problem is not with invalid time stamps; please read the comment
before the w32 wrapper.

> Even if Emacs can assume a "nice" ctime (a big assumption), it'd still
> have to do something like this:
> 
>   p = ctime (&timestamp);
>   if (! p)
>     fatal ("current time is out of range");
> 
> But notice that neither code snippet needs the w32-style wrapper; so
> the wrapper can be safely removed.

Paul, please re-read the comment in w32.c: it says that ctime returns
NULL when the current directory is on a networked drive.  So this has
nothing to do with invalid time stamps, and throwing a fatal error in
such cases would be a very bad idea, IMO.

I agree that the wrapper conceals the cases where ctime returns NULL
due to invalid time stamp, which is unfortunate.  But until someone
comes up with a simple enough solution that would differentiate
between the networked directory case and the invalid time case, I
think the wrapper is the lesser evil, especially since ``Emacs no
longer uses ctime'' for producing time strings (and thus the invalid
time stamp case is from now on much less important when ctime is
used).

> If these arguments convince you, then let's please install the
> above-mentioned change to the w32 files, which removes the ctime
> wrappers.  If not, then I suggest the following change instead.  It
> inserts a comment that attempts to explain the current situation, as I
> understand it.

I don't mind adding a comment, modulo the remarks below.

> It also corrects a minor problem with the wrappers:
> they return "Sun Jan 01 00:00:00 1970" for out-of-range time stamps,
> which is wrong for two reasons.  First, 1970-01-01 was a Thursday, not
> a Sunday.  Second, the day of the month should be space-padded, not
> zero-padded.

I guess "Sun Jan 01" was meant to emulate a zeroed out struct tm, so
I'm not so sure it's a problem.  But I have no objections to these
changes.

>   /* Place a wrapper around the MSVC version of ctime.  It returns NULL
>      on network directories, so we handle that case here.
> !    (Ulrich Leodolter, 1/11/95).
> ! 
> !    ctime has undefined behavior with out-of-range time stamps so Emacs
> !    no longer uses it.  However, a future version of Emacs might find a
> !    use for ctime, in a context where time stamps are known to be in a
> !    safe range for POSIX (i.e., from 1970 through 9999), but not for NT
> !    (1970 through 3000), so this wrapper has not been removed from the
> !    Emacs source code.  */

I do have an issue with this comment: it gives an impression that the
main reason for leaving the wrappers in place is the different range
of valid time stamps between Posix and MS-Windows systems.  But the
_real_ reason is the case of the network directories that is mentioned
above.  So how about the following comment instead:

  /* Place a wrapper around the MSVC version of ctime.  It returns NULL
     on network directories, so we handle that case here.
!    (Ulrich Leodolter, 1/11/95).
! 
!    (ctime has undefined behavior with out-of-range time stamps so Emacs
!    no longer uses it.  However, a future version of Emacs might find a
!    use for ctime, in a context where time stamps are known to be in a
!    safe range, so this wrapper has not been removed from the Emacs
!    source code, to avoid tripping on the network directory case.)  */

Note that I placed the whole addition in parentheses, since it really
is a kind of footnote for the original comment, which states the real
reason for having the wrapper.