Re: master d0c77a1: Remove some assumptions about timestamp format

[Michael Albinus]

Paul Eggert <address@hidden> writes:

Hi Paul,

> A constant like that would make things clearer, yes. Either 0 or (0 0
> 0 0) or (0 0) should do, so I suggest just 0 as it's simplest. While
> you're at it, you might consider changing the number from 0 to a
> negative value, say -1 (the value that POSIX uses for invalid time_t),
> since that's less likely to collide with actual file timestamps
> (timestamp 0 is far more common than timestamp -1 in real filesystems
> in my experience).

There is already invalid_timespec() in systime.h. What about the
following patch?

--8<---------------cut here---------------start------------->8---
diff --git a/src/editfns.c b/src/editfns.c
index 8c7491beed..fc076a52c1 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -1592,6 +1592,10 @@ static Lisp_Object
 time_arith (Lisp_Object a, Lisp_Object b,
            struct lisp_time (*op) (struct lisp_time, struct lisp_time))
 {
+  if (!NILP (Ftime_equal_p (a, Vinvalid_time)) ||
+      !NILP (Ftime_equal_p (b, Vinvalid_time)))
+    return Vinvalid_time;
+
   int alen, blen;
   struct lisp_time ta = lisp_time_struct (a, &alen);
   struct lisp_time tb = lisp_time_struct (b, &blen);
@@ -1620,6 +1624,7 @@ time_arith (Lisp_Object a, Lisp_Object b,
 DEFUN ("time-add", Ftime_add, Stime_add, 2, 2, 0,
        doc: /* Return the sum of two time values A and B, as a time value.
 A nil value for either argument stands for the current time.
+If time value A or B is equal to `invalid-time', this value is returned.
 See `current-time-string' for the various forms of a time value.  */)
   (Lisp_Object a, Lisp_Object b)
 {
@@ -1630,6 +1635,7 @@ DEFUN ("time-subtract", Ftime_subtract, Stime_subtract, 
2, 2, 0,
        doc: /* Return the difference between two time values A and B, as a 
time value.
 Use `float-time' to convert the difference into elapsed seconds.
 A nil value for either argument stands for the current time.
+If time value A or B is equal to `invalid-time', this value is returned.
 See `current-time-string' for the various forms of a time value.  */)
   (Lisp_Object a, Lisp_Object b)
 {
@@ -1652,6 +1658,19 @@ See `current-time-string' for the various forms of a 
time value.  */)
          ? Qt : Qnil);
 }
 
+DEFUN ("time-equal-p", Ftime_equal_p, Stime_equal_p, 2, 2, 0,
+       doc: /* Return non-nil if time value T1 is equal to time value T2.
+A nil value for either argument stands for the current time.
+See `current-time-string' for the various forms of a time value.  */)
+  (Lisp_Object t1, Lisp_Object t2)
+{
+  int t1len, t2len;
+  struct lisp_time a = lisp_time_struct (t1, &t1len);
+  struct lisp_time b = lisp_time_struct (t2, &t2len);
+  return
+    a.hi == b.hi && a.lo == b.lo && a.us == b.us && a.ps == b.ps ? Qt : Qnil;
+}
+
 
 DEFUN ("get-internal-run-time", Fget_internal_run_time, Sget_internal_run_time,
        0, 0, 0,
@@ -2287,6 +2306,9 @@ the TZ environment variable.  It can also be a list (as 
from
 without consideration for daylight saving time.  */)
   (Lisp_Object specified_time, Lisp_Object zone)
 {
+  if (Ftime_equal_p (specified_time, Vinvalid_time))
+      check_time_validity (0);
+
   time_t value = lisp_seconds_argument (specified_time);
   timezone_t tz = tzlookup (zone, false);
 
@@ -5661,6 +5683,10 @@ it to be non-nil.  */);
   binary_as_unsigned = true;
 #endif
 
+  DEFVAR_LISP ("invalid-time", Vinvalid_time,
+              doc: /* An invalid time value, used as "Don't know" value.  */);
+  Vinvalid_time = make_lisp_time (invalid_timespec ());
+
   defsubr (&Spropertize);
   defsubr (&Schar_equal);
   defsubr (&Sgoto_char);
@@ -5734,6 +5760,7 @@ it to be non-nil.  */);
   defsubr (&Stime_add);
   defsubr (&Stime_subtract);
   defsubr (&Stime_less_p);
+  defsubr (&Stime_equal_p);
   defsubr (&Sget_internal_run_time);
   defsubr (&Sformat_time_string);
   defsubr (&Sfloat_time);
--8<---------------cut here---------------end--------------->8---

> The Tramp code shouldn't care whether the constant is 0 or (0 0) or (0
> 0 0 0) [or -1 or (-1 65535) or (-1 65535 0 0)], because the code
> should compare time stamps via time-less-p (or float-time, if the
> timestamps are known to be small like 0 or -1, or maybe we should add
> time-equal?), not via 'equal'. The idea is that the Lisp timestamp
> format has changed in the past and is likely to change in the future
> and one should not assume any particular format if one wants the code
> to be portable.

Tramp would use only `invalid-time' and `time-equal-p', agnostic to
their implementations. Plus some compatibility code for older Emacsen.

Best regards, Michael.