Subject: Re: bug#9000: patch for higher-resolution time stamps

From: Paul Eggert <[email-masked]>
Date: Sat, 23 Jun 2012 21:31:38 -0700

On 06/23/2012 07:57 PM, Eli Zaretskii wrote:
> We could use (UINTMAX_MAX / 2) instead, would that work?

I don't think so, as the buggy compiler would still complain.

> The key to avoiding the warning is to have both arguments of 'min'
> be 64-bit values.

In my experience the width of the constant is irrelevant; what matters
is its value.  If an obsolete buggy GCC sees "E < C" where E is an
integer expression and C is a positive constant that is out of E's
type's range, it sometimes proudly reports that it is optimizing the
comparison away.  These warnings are typically bogus, and this was
considered a bug in GCC that was eventually fixed; the warnings
certainly are bogus for Emacs.

Normally we should not worry about silencing bogus warnings from
obsolete compilers, particularly when attempts to silence them are
liable to make the code less reliable or readable.  I suppose we can
make an exception here but I hope this doesn't establish a precedent.

I rummaged around our servers and found an old GCC that issues the
diagnostic, and installed as trunk bzr 108714 a further patch to silence
those particular warnings without introducing a signedness bug.
Here's our combined patch:

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-06-23 19:28:01 +0000
+++ src/ChangeLog	2012-06-24 04:11:19 +0000
@@ -1,3 +1,22 @@
+2012-06-24  Paul Eggert  <[email-masked]>
+
+	Fix bug when time_t is unsigned and as wide as intmax_t (Bug#9000).
+	* lisp.h (WAIT_READING_MAX): New macro.
+	* dispnew.c (Fsleep_for, sit_for):
+	* keyboard.c (kbd_buffer_get_event):
+	* process.c (Faccept_process_output):
+	Use it to avoid bogus compiler warnings with obsolescent GCC versions.
+	This improves on the previous patch, which introduced a bug
+	when time_t is unsigned and as wide as intmax_t.
+	See <http://bugs.gnu.org/9000#51>.
+
+2012-06-23  Eli Zaretskii  <[email-masked]>
+
+	* dispnew.c (sit_for, Fsleep_for):
+	* keyboard.c (kbd_buffer_get_event):
+	* process.c (Faccept_process_output): Avoid compiler warnings when
+	comparing a 32-bit time_t with a 64-bit INTMAX_MAX.
+
 2012-06-23  Juanma Barranquero  <[email-masked]>
 
 	* makefile.w32-in: Update dependencies.

=== modified file 'src/dispnew.c'
--- src/dispnew.c	2012-06-22 21:17:42 +0000
+++ src/dispnew.c	2012-06-24 04:11:19 +0000
@@ -5957,7 +5957,7 @@
   if (0 < duration)
     {
       EMACS_TIME t = EMACS_TIME_FROM_DOUBLE (duration);
-      wait_reading_process_output (min (EMACS_SECS (t), INTMAX_MAX),
+      wait_reading_process_output (min (EMACS_SECS (t), WAIT_READING_MAX),
 				   EMACS_NSECS (t), 0, 0, Qnil, NULL, 0);
     }
 
@@ -6005,7 +6005,7 @@
       else
 	{
 	  EMACS_TIME t = EMACS_TIME_FROM_DOUBLE (seconds);
-	  sec = min (EMACS_SECS (t), INTMAX_MAX);
+	  sec = min (EMACS_SECS (t), WAIT_READING_MAX);
 	  nsec = EMACS_NSECS (t);
 	}
     }

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2012-06-23 12:39:23 +0000
+++ src/keyboard.c	2012-06-24 04:11:19 +0000
@@ -3859,7 +3859,7 @@
 	    {
 	      EMACS_SUB_TIME (duration, *end_time, duration);
 	      wait_reading_process_output (min (EMACS_SECS (duration),
-						INTMAX_MAX),
+						WAIT_READING_MAX),
 					   EMACS_NSECS (duration),
 					   -1, 1, Qnil, NULL, 0);
 	    }

=== modified file 'src/lisp.h'
--- src/lisp.h	2012-06-22 21:17:42 +0000
+++ src/lisp.h	2012-06-24 04:11:19 +0000
@@ -3249,6 +3249,14 @@
                                         Lisp_Object,
                                         struct Lisp_Process *,
                                         int);
+/* Max value for the first argument of wait_reading_process_output.  */
+#if __GNUC__ == 3 || (__GNUC__ == 4 && __GNUC_MINOR__ <= 5)
+/* Work around a bug in GCC 3.4.2, known to be fixed in GCC 4.6.3.
+   The bug merely causes a bogus warning, but the warning is annoying.  */
+# define WAIT_READING_MAX min (TYPE_MAXIMUM (time_t), INTMAX_MAX)
+#else
+# define WAIT_READING_MAX INTMAX_MAX
+#endif
 extern void add_keyboard_wait_descriptor (int);
 extern void delete_keyboard_wait_descriptor (int);
 #ifdef HAVE_GPM

=== modified file 'src/process.c'
--- src/process.c	2012-06-22 21:17:42 +0000
+++ src/process.c	2012-06-24 04:11:19 +0000
@@ -3996,7 +3996,7 @@
 	  if (0 < XFLOAT_DATA (seconds))
 	    {
 	      EMACS_TIME t = EMACS_TIME_FROM_DOUBLE (XFLOAT_DATA (seconds));
-	      secs = min (EMACS_SECS (t), INTMAX_MAX);
+	      secs = min (EMACS_SECS (t), WAIT_READING_MAX);
 	      nsecs = EMACS_NSECS (t);
 	    }
 	}