[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Eli Zaretskii
Subject: Re: Emacs current-time-string core dump on 64-bit hosts
Date: Sat, 25 Mar 2006 11:10:46 +0200

> From: Paul Eggert <address@hidden>
> Date: Fri, 24 Mar 2006 12:45:02 -0800
> Cc: address@hidden
> Richard Stallman <address@hidden> writes:
> > Would you please put the new code directly into editfns.c?
> > That would avoid the need for new files, makefile changes, etc.
> OK, I've done this, and installed the following patch.  This patch is
> simpler than what I submitted earlier, since it uses asctime rather
> than formatting by hand.


> 2006-03-24  Paul Eggert  <address@hidden>
>       * editfns.c: Do not use ctime, since it has undefined behavior
>       with out-of-range time stamps.  This fixes a bug where
>       (current-time-string '(2814749767106 0)) would make Emacs dump
>       core on 64-bit Solaris 8.  The fix is to use localtime+asctime
>       (checking for in-range results) instead of ctime.  Please see
>       <http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>
>       for more details about this portability problem.
>       (TM_YEAR_BASE): Move up, so the changes below can use it.
>       (Fdecode_time, Fencode_time): Use TM_YEAR_BASE instead of 1900.
>       (Fdecode_time): Cast tm_year to EMACS_INT, to avoid overflow when
>       int is narrower than EMACS_INT.
>       (Fcurrent_time_string): As with Fformat_time_string, report an
>       invalid time specification if the argument is invalid.  Also,
>       check for out-of-range time stamps; this prevents a buffer overrun
>       that causes Emacs to dump core on 64-bit Solaris sparc, and it
>       preserves the historic behavior of always returning a fixed-size
>       string.

I took the liberty of moving the explanations in these entries to the
respective parts of the source.  I think such explanations belong in
the source, not in the logs.

>    if (! lisp_time_argument (specified_time, &value, NULL))
> -    value = -1;
> -  tem = (char *) ctime (&value);
> +    error ("Invalid time specification");
> +  tm = localtime (&value);
> +  if (! (tm && -999 - TM_YEAR_BASE <= tm->tm_year
> +      && tm->tm_year <= 9999 - TM_YEAR_BASE))
> +    error ("Specified time is not representable");
> +  tem = asctime (tm);

I'm a bit worried about this change: previously current-time-string
never threw an error, while now it will.  In particular, invalid
values of the optional argument will now cause incompatible behavior:
where previously current-time-string would pass -1 to ctime, typically
causing "(null)" be the result, it now signals an error.  Wouldn't it
be better to simply produce some telltale string instead, and not
throw an error?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]