unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
Cc: bug-gnu-emacs@gnu.org
Subject: Re: Emacs current-time-string core dump on 64-bit hosts
Date: Tue, 28 Mar 2006 12:16:25 +0200	[thread overview]
Message-ID: <u64lydgcm.fsf@gnu.org> (raw)
In-Reply-To: <878xqv1rbk.fsf@penguin.cs.ucla.edu> (message from Paul Eggert on Mon, 27 Mar 2006 14:00:15 -0800)

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

  reply	other threads:[~2006-03-28 10:16 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-17  5:58 Emacs current-time-string core dump on 64-bit hosts Paul Eggert
2006-03-17 12:16 ` Eli Zaretskii
2006-03-17 12:46   ` Andreas Schwab
2006-03-17 16:04   ` Kevin Rodgers
2006-03-18  0:44   ` Paul Eggert
2006-03-18 11:50     ` Eli Zaretskii
2006-03-19  2:30       ` Paul Eggert
2006-03-21 19:25         ` Richard Stallman
2006-03-18  8:43   ` Richard Stallman
2006-03-19  1:53     ` Paul Eggert
2006-03-19 21:50       ` Richard Stallman
2006-03-19 23:44         ` Paul Eggert
2006-03-20 19:59           ` Eli Zaretskii
2006-03-27 22:00             ` Paul Eggert
2006-03-28 10:16               ` Eli Zaretskii [this message]
2006-03-30  7:52                 ` Paul Eggert
2006-03-30 20:36                   ` Eli Zaretskii
2006-04-04  4:57                   ` Paul Eggert
2006-04-04 18:20                     ` Eli Zaretskii
2006-03-21  1:00           ` Richard Stallman
2006-03-24 20:45             ` Paul Eggert
2006-03-25  9:10               ` Eli Zaretskii
2006-03-26  5:25                 ` Paul Eggert
2006-03-26 20:06                   ` Eli Zaretskii
2006-03-27 22:29                     ` Richard Stallman
2006-03-28 10:20                       ` Eli Zaretskii
2006-03-29  8:14                         ` Richard Stallman
2006-03-25 15:26               ` Richard Stallman
2006-03-24 21:00             ` Paul Eggert
2006-03-24 21:09             ` Paul Eggert
2006-03-25 15:26               ` Richard Stallman
2006-03-26  7:31                 ` Paul Eggert
     [not found]                   ` <E1FNnCd-0000pN-J4@fencepost.gnu.org>
2006-03-27 20:49                     ` Paul Eggert
2006-03-28 19:33                       ` Richard Stallman
2006-03-30  7:57                         ` Paul Eggert
2006-03-31 17:28                           ` Richard Stallman
2006-03-31 20:51                             ` Paul Eggert
2006-04-01 20:28                               ` Richard Stallman
2006-04-03  4:44                                 ` Paul Eggert
2006-03-17 16:25 ` Andreas Schwab
  -- strict thread matches above, loose matches on Subject: below --
2006-03-17  8:02 Paul Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=u64lydgcm.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=bug-gnu-emacs@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).