bug#8664: * keyboard.c (make_lispy_event): Fix problem in integer overflow.

bug#8664: * keyboard.c (make_lispy_event): Fix problem in integer overflow.

[Paul Eggert]

Here's a patch for a potential problem with integer overflow
on 64-bit hosts that I plan to install after some more testing.
The problem is a bit more severe if EMACS_INT is 64-bit on
a 32-bit host, and I found it by inspection.

* keyboard.c (make_lispy_event): Fix problem in integer overflow.
Don't assume that the difference between two unsigned long values
can fit into an integer.  At this point, we know button_down_time
<= event->timestamp, so the difference must be nonnegative, so
there's no need to cast the result if double-click-time is
nonnegative, as it should be; check that it's nonnegative, just in
case.  This bug is triggered when events are more than 2**31 ms
apart (about 25 days).
=== modified file 'src/keyboard.c'
--- src/keyboard.c	2011-04-28 19:35:20 +0000
+++ src/keyboard.c	2011-05-12 19:33:15 +0000
@@ -5556,9 +5556,9 @@
 		       && (eabs (XINT (event->y) - last_mouse_y) <= fuzz)
 		       && button_down_time != 0
 		       && (EQ (Vdouble_click_time, Qt)
-			   || (INTEGERP (Vdouble_click_time)
-			       && ((int)(event->timestamp - button_down_time)
-				   < XINT (Vdouble_click_time)))));
+			   || (NATNUMP (Vdouble_click_time)
+			       && (event->timestamp - button_down_time
+				   < XFASTINT (Vdouble_click_time)))));
 	}

 	last_mouse_button = button;
@@ -5742,9 +5742,9 @@
 		       && (eabs (XINT (event->y) - last_mouse_y) <= fuzz)
 		       && button_down_time != 0
 		       && (EQ (Vdouble_click_time, Qt)
-			   || (INTEGERP (Vdouble_click_time)
-			       && ((int)(event->timestamp - button_down_time)
-				   < XINT (Vdouble_click_time)))));
+			   || (NATNUMP (Vdouble_click_time)
+			       && (event->timestamp - button_down_time
+				   < XFASTINT (Vdouble_click_time)))));
 	  if (is_double)
 	    {
 	      double_click_count++;

bug#8664: Being more-systematic about user-interface timestamps

[Paul Eggert]

Here's a further patch, to make it easier to catch bugs like
Bug#8664 in the future.


Be more systematic about user-interface timestamps.
Before, the code sometimes used 'Time', sometimes 'unsigned long',
and sometimes 'EMACS_UINT', to represent these timestamps.  This
change causes it to use 'Time' uniformly, as that's what X uses.
This makes the code easier to follow, and makes it easier to catch
integer overflow bugs such as Bug#8664.
* frame.c (Fmouse_position, Fmouse_pixel_position):
Use Time, not unsigned long, for user-interface timestamps.
* keyboard.c (last_event_timestamp, kbd_buffer_get_event): Likewise.
(button_down_time, make_lispy_position, make_lispy_movement): Likewise.
* keyboard.h (last_event_timestamp): Likewise.
* menu.c (Fx_popup_menu) [!HAVE_X_WINDOWS]: Likewise.
* menu.h (xmenu_show): Likewise.
* term.c (term_mouse_position): Likewise.
* termhooks.h (struct input_event.timestamp): Likewise.
(struct terminal.mouse_position_hook): Likewise.
* xmenu.c (create_and_show_popup_menu, xmenu_show): Likewise.
* xterm.c (XTmouse_position, x_scroll_bar_report_motion): Likewise.
* systime.h (Time): New decl.  Pull it in from <X11/X.h> if
HAVE_X_WINDOWS, otherwise define it as unsigned long, which is
what it was before.
* menu.h, termhooks.h: Include "systime.h", for Time.
=== modified file 'src/frame.c'
--- src/frame.c	2011-04-19 01:15:59 +0000
+++ src/frame.c	2011-05-12 17:15:05 +0000
@@ -1631,7 +1631,7 @@
   enum scroll_bar_part party_dummy;
   Lisp_Object x, y, retval;
   int col, row;
-  unsigned long long_dummy;
+  Time long_dummy;
   struct gcpro gcpro1;

   f = SELECTED_FRAME ();
@@ -1676,7 +1676,7 @@
   Lisp_Object lispy_dummy;
   enum scroll_bar_part party_dummy;
   Lisp_Object x, y;
-  unsigned long long_dummy;
+  Time long_dummy;

   f = SELECTED_FRAME ();
   x = y = Qnil;

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2011-05-12 19:37:40 +0000
+++ src/keyboard.c	2011-05-12 19:59:08 +0000
@@ -238,7 +238,7 @@

 /* The timestamp of the last input event we received from the X server.
    X Windows wants this for selection ownership.  */
-unsigned long last_event_timestamp;
+Time last_event_timestamp;

 static Lisp_Object Qx_set_selection, Qhandle_switch_frame;
 Lisp_Object QPRIMARY;
@@ -4085,7 +4085,7 @@
       Lisp_Object bar_window;
       enum scroll_bar_part part;
       Lisp_Object x, y;
-      unsigned long t;
+      Time t;

       *kbp = current_kboard;
       /* Note that this uses F to determine which terminal to look at.
@@ -5088,7 +5088,7 @@
 static int last_mouse_button;
 static int last_mouse_x;
 static int last_mouse_y;
-static unsigned long button_down_time;
+static Time button_down_time;

 /* The number of clicks in this multiple-click. */

@@ -5099,7 +5099,7 @@

 static Lisp_Object
 make_lispy_position (struct frame *f, Lisp_Object x, Lisp_Object y,
-		     unsigned long t)
+		     Time t)
 {
   enum window_part part;
   Lisp_Object posn = Qnil;
@@ -5987,7 +5987,7 @@

 static Lisp_Object
 make_lispy_movement (FRAME_PTR frame, Lisp_Object bar_window, enum scroll_bar_part part,
-		     Lisp_Object x, Lisp_Object y, unsigned long t)
+		     Lisp_Object x, Lisp_Object y, Time t)
 {
   /* Is it a scroll bar movement?  */
   if (frame && ! NILP (bar_window))

=== modified file 'src/keyboard.h'
--- src/keyboard.h	2011-04-14 01:36:53 +0000
+++ src/keyboard.h	2011-05-12 17:08:48 +0000
@@ -16,7 +16,7 @@
 You should have received a copy of the GNU General Public License
 along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */

-#include "systime.h"		/* for EMACS_TIME */
+#include "systime.h"		/* for EMACS_TIME, Time */
 #include "coding.h"             /* for ENCODE_UTF_8 and ENCODE_SYSTEM */

 /* Lisp fields in struct keyboard are hidden from most code and accessed
@@ -459,7 +459,7 @@

 /* The timestamp of the last input event we received from the X server.
    X Windows wants this for selection ownership.  */
-extern unsigned long last_event_timestamp;
+extern Time last_event_timestamp;

 extern int quit_char;


=== modified file 'src/menu.c'
--- src/menu.c	2011-05-12 06:48:32 +0000
+++ src/menu.c	2011-05-12 16:55:07 +0000
@@ -1147,13 +1147,13 @@
 #else /* not HAVE_X_WINDOWS */
 	Lisp_Object bar_window;
 	enum scroll_bar_part part;
-	unsigned long time;
+	Time time;
         void (*mouse_position_hook) (struct frame **, int,
                                      Lisp_Object *,
                                      enum scroll_bar_part *,
                                      Lisp_Object *,
                                      Lisp_Object *,
-                                     unsigned long *) =
+                                     Time *) =
 	  FRAME_TERMINAL (new_f)->mouse_position_hook;

 	if (mouse_position_hook)

=== modified file 'src/menu.h'
--- src/menu.h	2011-01-25 04:08:28 +0000
+++ src/menu.h	2011-05-12 20:09:02 +0000
@@ -19,6 +19,8 @@
 #ifndef MENU_H
 #define MENU_H

+#include "systime.h" /* for Time */
+
 extern void x_set_menu_bar_lines (struct frame *f,
                                   Lisp_Object value,
                                   Lisp_Object oldval);
@@ -48,6 +50,5 @@
 extern Lisp_Object ns_menu_show (FRAME_PTR, int, int, int, int,
 				 Lisp_Object, const char **);
 extern Lisp_Object xmenu_show (FRAME_PTR, int, int, int, int,
-			       Lisp_Object, const char **, EMACS_UINT);
+			       Lisp_Object, const char **, Time);
 #endif /* MENU_H */
-

=== modified file 'src/systime.h'
--- src/systime.h	2011-03-11 20:24:09 +0000
+++ src/systime.h	2011-05-12 17:07:49 +0000
@@ -30,6 +30,12 @@
 #endif
 #endif

+#ifdef HAVE_X_WINDOWS
+# include <X11/X.h>
+#else
+typedef unsigned long Time;
+#endif
+
 #ifdef HAVE_TZNAME
 #ifndef tzname		/* For SGI.  */
 extern char *tzname[];	/* RS6000 and others want it this way.  */

=== modified file 'src/term.c'
--- src/term.c	2011-05-04 07:20:46 +0000
+++ src/term.c	2011-05-12 20:16:21 +0000
@@ -2698,7 +2698,7 @@
 static void
 term_mouse_position (FRAME_PTR *fp, int insist, Lisp_Object *bar_window,
 		     enum scroll_bar_part *part, Lisp_Object *x,
-		     Lisp_Object *y, unsigned long *timeptr)
+		     Lisp_Object *y, Time *timeptr)
 {
   struct timeval now;


=== modified file 'src/termhooks.h'
--- src/termhooks.h	2011-04-25 19:40:22 +0000
+++ src/termhooks.h	2011-05-12 17:13:37 +0000
@@ -20,6 +20,8 @@
 \f
 /* Miscellanea.   */

+#include "systime.h" /* for Time */
+
 struct glyph;
 struct frame;
 \f
@@ -233,7 +235,7 @@
   int modifiers;		/* See enum below for interpretation.  */

   Lisp_Object x, y;
-  unsigned long timestamp;
+  Time timestamp;

   /* This is padding just to put the frame_or_window field
      past the size of struct selection_input_event.  */
@@ -463,7 +465,7 @@
                                enum scroll_bar_part *part,
                                Lisp_Object *x,
                                Lisp_Object *y,
-                               unsigned long *);
+                               Time *);

   /* The window system handling code should set this if the mouse has
      moved since the last call to the mouse_position_hook.  Calling that

=== modified file 'src/xmenu.c'
--- src/xmenu.c	2011-05-12 16:16:40 +0000
+++ src/xmenu.c	2011-05-12 16:32:07 +0000
@@ -240,7 +240,7 @@
       FRAME_PTR new_f = SELECTED_FRAME ();
       Lisp_Object bar_window;
       enum scroll_bar_part part;
-      unsigned long time;
+      Time time;
       Lisp_Object x, y;

       (*mouse_position_hook) (&new_f, 1, &bar_window, &part, &x, &y, &time);
@@ -1420,7 +1420,8 @@
    menu pops down.
    menu_item_selection will be set to the selection.  */
 static void
-create_and_show_popup_menu (FRAME_PTR f, widget_value *first_wv, int x, int y, int for_click, EMACS_UINT timestamp)
+create_and_show_popup_menu (FRAME_PTR f, widget_value *first_wv, int x, int y,
+			    int for_click, Time timestamp)
 {
   int i;
   GtkWidget *menu;
@@ -1464,7 +1465,7 @@
   gtk_widget_show_all (menu);

   gtk_menu_popup (GTK_MENU (menu), 0, 0, pos_func, &popup_x_y, i,
-		  timestamp > 0 ? timestamp : gtk_get_current_event_time());
+		  timestamp ? timestamp : gtk_get_current_event_time ());

   record_unwind_protect (pop_down_menu, make_save_value (menu, 0));

@@ -1524,7 +1525,7 @@
    menu_item_selection will be set to the selection.  */
 static void
 create_and_show_popup_menu (FRAME_PTR f, widget_value *first_wv,
-			    int x, int y, int for_click, EMACS_UINT timestamp)
+			    int x, int y, int for_click, Time timestamp)
 {
   int i;
   Arg av[2];
@@ -1598,7 +1599,7 @@

 Lisp_Object
 xmenu_show (FRAME_PTR f, int x, int y, int for_click, int keymaps,
-	    Lisp_Object title, const char **error_name, EMACS_UINT timestamp)
+	    Lisp_Object title, const char **error_name, Time timestamp)
 {
   int i;
   widget_value *wv, *save_wv = 0, *first_wv = 0, *prev_wv = 0;
@@ -2241,7 +2242,7 @@

 Lisp_Object
 xmenu_show (FRAME_PTR f, int x, int y, int for_click, int keymaps,
-	    Lisp_Object title, const char **error_name, EMACS_UINT timestamp)
+	    Lisp_Object title, const char **error_name, Time timestamp)
 {
   Window root;
   XMenu *menu;

=== modified file 'src/xterm.c'
--- src/xterm.c	2011-05-11 23:16:52 +0000
+++ src/xterm.c	2011-05-12 17:01:31 +0000
@@ -342,7 +342,7 @@
 static void x_scroll_bar_report_motion (struct frame **, Lisp_Object *,
                                         enum scroll_bar_part *,
                                         Lisp_Object *, Lisp_Object *,
-                                        unsigned long *);
+                                        Time *);
 static void x_handle_net_wm_state (struct frame *, XPropertyEvent *);
 static void x_check_fullscreen (struct frame *);
 static void x_check_expected_move (struct frame *, int, int);
@@ -3799,7 +3799,7 @@
 static void
 XTmouse_position (FRAME_PTR *fp, int insist, Lisp_Object *bar_window,
 		  enum scroll_bar_part *part, Lisp_Object *x, Lisp_Object *y,
-		  long unsigned int *timestamp)
+		  Time *timestamp)
 {
   FRAME_PTR f1;

@@ -5534,7 +5534,7 @@
 static void
 x_scroll_bar_report_motion (FRAME_PTR *fp, Lisp_Object *bar_window,
 			    enum scroll_bar_part *part, Lisp_Object *x,
-			    Lisp_Object *y, long unsigned int *timestamp)
+			    Lisp_Object *y, Time *timestamp)
 {
   struct scroll_bar *bar = XSCROLL_BAR (last_mouse_scroll_bar);
   Window w = bar->x_window;

bug#8664: Being more-systematic about user-interface timestamps

[Eli Zaretskii]

> Date: Thu, 12 May 2011 13:26:06 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> --- src/systime.h	2011-03-11 20:24:09 +0000
> +++ src/systime.h	2011-05-12 17:07:49 +0000
> @@ -30,6 +30,12 @@
>  #endif
>  #endif
> 
> +#ifdef HAVE_X_WINDOWS
> +# include <X11/X.h>
> +#else
> +typedef unsigned long Time;
> +#endif

Wouldn't this clash with the typedef in w32gui.h?

> --- src/menu.c	2011-05-12 06:48:32 +0000
> +++ src/menu.c	2011-05-12 16:55:07 +0000
> @@ -1147,13 +1147,13 @@
>  #else /* not HAVE_X_WINDOWS */
>  	Lisp_Object bar_window;
>  	enum scroll_bar_part part;
> -	unsigned long time;
> +	Time time;
>          void (*mouse_position_hook) (struct frame **, int,
>                                       Lisp_Object *,
>                                       enum scroll_bar_part *,
>                                       Lisp_Object *,
>                                       Lisp_Object *,
> -                                     unsigned long *) =
> +                                     Time *) =

This needs a corresponding change in all the functions used as
mouse_position_hook on different platforms.  You made such a change
only in 2 of them: term_mouse_position and XTmouse_position.

bug#8664: Being more-systematic about user-interface timestamps

[Paul Eggert]

On 05/13/11 01:53, Eli Zaretskii wrote:
>> +#ifdef HAVE_X_WINDOWS
>> +# include <X11/X.h>
>> +#else
>> +typedef unsigned long Time;
>> +#endif
> 
> Wouldn't this clash with the typedef in w32gui.h?

Yes, thanks, good catch.  Proposed fix below.

> This needs a corresponding change in all the functions used as
> mouse_position_hook on different platforms.  You made such a change
> only in 2 of them: term_mouse_position and XTmouse_position.

Thanks for that too.  For NextStep a change is needed, also proposed below.

No change should be needed for w32's hooks, since there's no actual change
to the data type there.  For documentation purposes it might be nice to
run through the hooks and change 'unsigned long' to 'Time' where
appropriate, but that's not essential.  Normally I'm reluctant to mess
with the w32 code as I can't easily test it.


Fixups, following up to the user-interface timestamp change.
* nsterm.m (last_mouse_movement_time, ns_mouse_position): Use Time
for UI timestamps, instead of unsigned long.
* w32gui.h (Time): Define by including "systime.h" rather than by
declaring it ourselves.  (Bug#8664)
=== modified file 'src/nsterm.m'
--- src/nsterm.m	2011-04-03 08:30:57 +0000
+++ src/nsterm.m	2011-05-14 08:56:08 +0000
@@ -158,7 +158,7 @@
 /* display update */
 NSPoint last_mouse_motion_position;
 static NSRect last_mouse_glyph;
-static unsigned long last_mouse_movement_time = 0;
+static Time last_mouse_movement_time = 0;
 static Lisp_Object last_mouse_motion_frame;
 static EmacsScroller *last_mouse_scroll_bar = nil;
 static struct frame *ns_updating_frame;
@@ -1789,7 +1789,7 @@
 static void
 ns_mouse_position (struct frame **fp, int insist, Lisp_Object *bar_window,
                    enum scroll_bar_part *part, Lisp_Object *x, Lisp_Object *y,
-                   unsigned long *time)
+                   Time *time)
 /* --------------------------------------------------------------------------
     External (hook): inform emacs about mouse position and hit parts.
     If a scrollbar is being dragged, set bar_window, part, x, y, time.
@@ -6531,5 +6531,3 @@
   /* Tell emacs about this window system. */
   Fprovide (intern ("ns"), Qnil);
 }
-
-

=== modified file 'src/w32gui.h'
--- src/w32gui.h	2011-01-25 04:08:28 +0000
+++ src/w32gui.h	2011-05-14 09:01:32 +0000
@@ -20,6 +20,8 @@
 #define EMACS_W32GUI_H
 #include <windows.h>

+#include "systime.h" /* for Time */
+
 /* Local memory management for menus.  */
 #define local_heap (GetProcessHeap ())
 #define local_alloc(n) (HeapAlloc (local_heap, HEAP_ZERO_MEMORY, (n)))
@@ -47,7 +49,6 @@

 typedef XGCValues * GC;
 typedef COLORREF Color;
-typedef DWORD Time;
 typedef HWND Window;
 typedef HDC Display;  /* HDC so it doesn't conflict with xpm lib.  */
 typedef HCURSOR Cursor;
@@ -147,4 +148,3 @@


 #endif /* EMACS_W32GUI_H */
-

bug#8664: Being more-systematic about user-interface timestamps

[Eli Zaretskii]

> Date: Sat, 14 May 2011 02:10:27 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 8664@debbugs.gnu.org
> 
> No change should be needed for w32's hooks, since there's no actual change
> to the data type there.

It will break if the typedef is ever changed to be incompatible.

> For documentation purposes it might be nice to run through the hooks
> and change 'unsigned long' to 'Time' where appropriate, but that's
> not essential.  Normally I'm reluctant to mess with the w32 code as
> I can't easily test it.

Changing a single declaration to an obviously correct one, and an
equivalent one at that, can hardly be qualified as "messing".

Besides, people will understand if you botch their build trying to
keep it from breaking, but may not understand why you knowingly
refrain from making a change that clearly is TRT.

bug#8664: Being more-systematic about user-interface timestamps

[Paul Eggert]

On 05/14/11 02:41, Eli Zaretskii wrote:
> people will understand if you botch their build trying to
> keep it from breaking

Sure, but in this case the change cannot possibly fix anything,
and might break the build, which is why I omitted it.  But since
you're saying it's OK I'll fold the following change in when
I merge.

* msdos.c (mouse_get_pos): Likewise.
* w32inevt.c (movement_time, w32_console_mouse_position): Likewise.
=== modified file 'src/msdos.c'
--- src/msdos.c	2011-04-24 12:48:30 +0000
+++ src/msdos.c	2011-05-14 19:00:49 +0000
@@ -287,7 +287,7 @@
 void
 mouse_get_pos (FRAME_PTR *f, int insist, Lisp_Object *bar_window,
 	       enum scroll_bar_part *part, Lisp_Object *x, Lisp_Object *y,
-	       unsigned long *time)
+	       Time *time)
 {
   int ix, iy;
   Lisp_Object frame, tail;

=== modified file 'src/w32inevt.c'
--- src/w32inevt.c	2011-03-25 15:39:59 +0000
+++ src/w32inevt.c	2011-05-14 19:01:44 +0000
@@ -45,7 +45,7 @@

 /* Info for last mouse motion */
 static COORD movement_pos;
-static DWORD movement_time;
+static Time movement_time;

 /* from w32fns.c */
 extern unsigned int map_keypad_keys (unsigned int, unsigned int);
@@ -544,7 +544,7 @@
 			    enum scroll_bar_part *part,
 			    Lisp_Object *x,
 			    Lisp_Object *y,
-			    unsigned long *time)
+			    Time *time)
 {
   BLOCK_INPUT;

@@ -756,4 +756,3 @@
   UNBLOCK_INPUT;
   return ret;
 }
-

bug#8664: Being more-systematic about user-interface timestamps

[Eli Zaretskii]

> Date: Sat, 14 May 2011 12:09:29 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 8664@debbugs.gnu.org
> 
> On 05/14/11 02:41, Eli Zaretskii wrote:
> > people will understand if you botch their build trying to
> > keep it from breaking
> 
> Sure, but in this case the change cannot possibly fix anything,
> and might break the build, which is why I omitted it.  But since
> you're saying it's OK I'll fold the following change in when
> I merge.

Thanks.

bug#8675: lisp_string_width and strings wider than INT_MAX

[Paul Eggert]

lisp_string_width requires that the precision be less than INT_MAX,
and if no precision is given, misbehaves badly when dealing with
strings whose widths are greater than INT_MAX.  To address this, I
plan to install the following patch (PATCH 3) after some more testing.
PATCH 3 depends on two obvious patches: PATCH 2 introduces a helper
no-return function string_overflow, and PATCH 1 updates to the latest
version of gnulib.

One thing about PATCH 3: when used in Emacs, the new lisp_string_width
correctly signals an overflow when the string's width cannot be
represented, but when used outside Emacs (in mulelib) it silently
ignores the problem, just as before.  I don't know much about mulelib
and so don't know how to address this.  But anyway, mulelib uses
should be no worse off than before.

PATCH 3 ---------------------------------------------------
=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-05-16 01:11:54 +0000
+++ src/ChangeLog	2011-05-16 01:25:38 +0000
@@ -1,5 +1,14 @@
 2011-05-16  Paul Eggert  <eggert@cs.ucla.edu>
 
+	* character.c (lisp_string_width): Check for string overflow.
+	Use EMACS_INT, not int, for string indexes and lengths; in
+	particular, 2nd arg is now EMACS_INT, not int.  Do not crash if
+	the resulting string length overflows an EMACS_INT; instead,
+	report a string overflow if no precision given.  When checking for
+	precision exhaustion, use a check that cannot possibly have
+	integer overflow.
+	* character.h (lisp_string_width): Adjust to new signature.
+
 	* alloc.c (string_overflow): New function.
 	(Fmake_string): Use it.  This doesn't change behavior, but saves
 	a few bytes and will simplify future changes.

=== modified file 'src/character.c'
--- src/character.c	2011-05-16 01:11:54 +0000
+++ src/character.c	2011-05-16 01:17:19 +0000
@@ -35,6 +35,7 @@
 
 #include <sys/types.h>
 #include <setjmp.h>
+#include <intprops.h>
 #include "lisp.h"
 #include "character.h"
 #include "buffer.h"
@@ -404,7 +405,7 @@
    in *NCHARS and *NBYTES respectively.  */
 
 EMACS_INT
-lisp_string_width (Lisp_Object string, int precision,
+lisp_string_width (Lisp_Object string, EMACS_INT precision,
 		   EMACS_INT *nchars, EMACS_INT *nbytes)
 {
   EMACS_INT len = SCHARS (string);
@@ -419,7 +420,7 @@
 
   while (i < len)
     {
-      int chars, bytes, thiswidth;
+      EMACS_INT chars, bytes, thiswidth;
       Lisp_Object val;
       int cmp_id;
       EMACS_INT ignore, end;
@@ -437,7 +438,11 @@
 	  int c;
 
 	  if (multibyte)
-	    c = STRING_CHAR_AND_LENGTH (str + i_byte, bytes);
+	    {
+	      int cbytes;
+	      c = STRING_CHAR_AND_LENGTH (str + i_byte, cbytes);
+	      bytes = cbytes;
+	    }
 	  else
 	    c = str[i_byte], bytes = 1;
 	  chars = 1;
@@ -455,8 +460,14 @@
 	    }
 	}
 
-      if (precision > 0
-	  && (width + thiswidth > precision))
+      if (precision <= 0)
+	{
+#ifdef emacs
+	  if (INT_ADD_OVERFLOW (width, thiswidth))
+	    string_overflow ();
+#endif
+	}
+      else if (precision - width < thiswidth)
 	{
 	  *nchars = i;
 	  *nbytes = i_byte;

=== modified file 'src/character.h'
--- src/character.h	2011-04-11 03:39:45 +0000
+++ src/character.h	2011-05-16 01:13:28 +0000
@@ -612,7 +612,7 @@
 extern EMACS_INT strwidth (const char *, EMACS_INT);
 extern EMACS_INT c_string_width (const unsigned char *, EMACS_INT, int,
 				 EMACS_INT *, EMACS_INT *);
-extern EMACS_INT lisp_string_width (Lisp_Object, int,
+extern EMACS_INT lisp_string_width (Lisp_Object, EMACS_INT,
 				    EMACS_INT *, EMACS_INT *);
 
 extern Lisp_Object Qcharacterp;

PATCH 2 ---------------------------------------------------
=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-05-15 17:17:44 +0000
+++ src/ChangeLog	2011-05-16 01:11:54 +0000
@@ -1,3 +1,11 @@
+2011-05-16  Paul Eggert  <eggert@cs.ucla.edu>
+
+	* alloc.c (string_overflow): New function.
+	(Fmake_string): Use it.  This doesn't change behavior, but saves
+	a few bytes and will simplify future changes.
+	* character.c (string_escape_byte8): Likewise.
+	* lisp.h (string_overflow): New decl.
+
 2011-05-15  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Fixups, following up to the user-interface timestamp change.

=== modified file 'src/alloc.c'
--- src/alloc.c	2011-05-12 07:07:06 +0000
+++ src/alloc.c	2011-05-16 01:11:54 +0000
@@ -2174,6 +2174,11 @@
   current_sblock = tb;
 }
 
+void
+string_overflow (void)
+{
+  error ("Maximum string size exceeded");
+}
 
 DEFUN ("make-string", Fmake_string, Smake_string, 2, 2, 0,
        doc: /* Return a newly created string of length LENGTH, with INIT in each element.
@@ -2206,7 +2211,7 @@
       EMACS_INT string_len = XINT (length);
 
       if (string_len > MOST_POSITIVE_FIXNUM / len)
-	error ("Maximum string size exceeded");
+	string_overflow ();
       nbytes = len * string_len;
       val = make_uninit_multibyte_string (string_len, nbytes);
       p = SDATA (val);

=== modified file 'src/character.c'
--- src/character.c	2011-05-12 07:07:06 +0000
+++ src/character.c	2011-05-16 01:11:54 +0000
@@ -823,7 +823,7 @@
     {
       if ((MOST_POSITIVE_FIXNUM - nchars) / 3 < byte8_count
 	  || (MOST_POSITIVE_FIXNUM - nbytes) / 2 < byte8_count)
-	error ("Maximum string size exceeded");
+	string_overflow ();
 
       /* Convert 2-byte sequence of byte8 chars to 4-byte octal.  */
       val = make_uninit_multibyte_string (nchars + byte8_count * 3,
@@ -832,7 +832,7 @@
   else
     {
       if ((MOST_POSITIVE_FIXNUM - nchars) / 3 < byte8_count)
-	error ("Maximum string size exceeded");
+	string_overflow ();
       /* Convert 1-byte sequence of byte8 chars to 4-byte octal.  */
       val = make_uninit_string (nbytes + byte8_count * 3);
     }

=== modified file 'src/lisp.h'
--- src/lisp.h	2011-05-15 17:17:44 +0000
+++ src/lisp.h	2011-05-16 01:11:54 +0000
@@ -2710,6 +2710,7 @@
 EXFUN (Fvector, MANY);
 EXFUN (Fmake_symbol, 1);
 EXFUN (Fmake_marker, 0);
+extern void string_overflow (void) NO_RETURN;
 EXFUN (Fmake_string, 2);
 extern Lisp_Object build_string (const char *);
 extern Lisp_Object make_string (const char *, EMACS_INT);

PATCH 1 ---------------------------------------------------
=== modified file 'doc/misc/texinfo.tex'
--- doc/misc/texinfo.tex	2011-04-06 05:07:27 +0000
+++ doc/misc/texinfo.tex	2011-05-16 00:51:54 +0000
@@ -3,7 +3,7 @@
 % Load plain if necessary, i.e., if running under initex.
 \expandafter\ifx\csname fmtname\endcsname\relax\input plain\fi
 %
-\def\texinfoversion{2011-03-25.11}
+\def\texinfoversion{2011-05-11.16}
 %
 % Copyright 1985, 1986, 1988, 1990, 1991, 1992, 1993, 1994, 1995,
 % 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006,
@@ -8424,7 +8424,7 @@
 %
 % Latin1 (ISO-8859-1) character definitions.
 \def\latonechardefs{%
-  \gdef^^a0{~}
+  \gdef^^a0{\tie}
   \gdef^^a1{\exclamdown}
   \gdef^^a2{\missingcharmsg{CENT SIGN}}
   \gdef^^a3{{\pounds}}
@@ -8546,7 +8546,7 @@
 
 % Latin2 (ISO-8859-2) character definitions.
 \def\lattwochardefs{%
-  \gdef^^a0{~}
+  \gdef^^a0{\tie}
   \gdef^^a1{\ogonek{A}}
   \gdef^^a2{\u{}}
   \gdef^^a3{\L}
@@ -9395,6 +9395,8 @@
 
 \message{and turning on texinfo input format.}
 
+\def^^L{\par} % remove \outer, so ^L can appear in an @comment
+
 % DEL is a comment character, in case @c does not suffice.
 \catcode`\^^? = 14
 

=== modified file 'lib/gnulib.mk'
--- lib/gnulib.mk	2011-05-04 07:19:21 +0000
+++ lib/gnulib.mk	2011-05-16 00:51:54 +0000
@@ -167,7 +167,8 @@
 
 ## begin gnulib module ignore-value
 
-libgnu_a_SOURCES += ignore-value.h
+
+EXTRA_DIST += ignore-value.h
 
 ## end   gnulib module ignore-value
 
@@ -779,6 +780,7 @@
 	      -e 's|@''GNULIB_GETLOGIN_R''@|$(GNULIB_GETLOGIN_R)|g' \
 	      -e 's|@''GNULIB_GETPAGESIZE''@|$(GNULIB_GETPAGESIZE)|g' \
 	      -e 's|@''GNULIB_GETUSERSHELL''@|$(GNULIB_GETUSERSHELL)|g' \
+	      -e 's|@''GNULIB_GROUP_MEMBER''@|$(GNULIB_GROUP_MEMBER)|g' \
 	      -e 's|@''GNULIB_LCHOWN''@|$(GNULIB_LCHOWN)|g' \
 	      -e 's|@''GNULIB_LINK''@|$(GNULIB_LINK)|g' \
 	      -e 's|@''GNULIB_LINKAT''@|$(GNULIB_LINKAT)|g' \
@@ -817,6 +819,7 @@
 	      -e 's|@''HAVE_GETHOSTNAME''@|$(HAVE_GETHOSTNAME)|g' \
 	      -e 's|@''HAVE_GETLOGIN''@|$(HAVE_GETLOGIN)|g' \
 	      -e 's|@''HAVE_GETPAGESIZE''@|$(HAVE_GETPAGESIZE)|g' \
+	      -e 's|@''HAVE_GROUP_MEMBER''@|$(HAVE_GROUP_MEMBER)|g' \
 	      -e 's|@''HAVE_LCHOWN''@|$(HAVE_LCHOWN)|g' \
 	      -e 's|@''HAVE_LINK''@|$(HAVE_LINK)|g' \
 	      -e 's|@''HAVE_LINKAT''@|$(HAVE_LINKAT)|g' \
@@ -883,9 +886,10 @@
 ## begin gnulib module verify
 
 if gl_GNULIB_ENABLED_verify
-libgnu_a_SOURCES += verify.h
 
 endif
+EXTRA_DIST += verify.h
+
 ## end   gnulib module verify
 
 ## begin gnulib module warn-on-use

=== modified file 'lib/intprops.h'
--- lib/intprops.h	2011-01-30 19:22:02 +0000
+++ lib/intprops.h	2011-05-16 00:51:54 +0000
@@ -17,70 +17,298 @@
 
 /* Written by Paul Eggert.  */
 
-#ifndef GL_INTPROPS_H
-# define GL_INTPROPS_H
-
-# include <limits.h>
+#ifndef _GL_INTPROPS_H
+#define _GL_INTPROPS_H
+
+#include <limits.h>
+
+/* Return a integer value, converted to the same type as the integer
+   expression E after integer type promotion.  V is the unconverted value.
+   E should not have side effects.  */
+#define _GL_INT_CONVERT(e, v) ((e) - (e) + (v))
 
 /* The extra casts in the following macros work around compiler bugs,
    e.g., in Cray C 5.0.3.0.  */
 
 /* True if the arithmetic type T is an integer type.  bool counts as
    an integer.  */
-# define TYPE_IS_INTEGER(t) ((t) 1.5 == 1)
+#define TYPE_IS_INTEGER(t) ((t) 1.5 == 1)
 
 /* True if negative values of the signed integer type T use two's
    complement, ones' complement, or signed magnitude representation,
    respectively.  Much GNU code assumes two's complement, but some
    people like to be portable to all possible C hosts.  */
-# define TYPE_TWOS_COMPLEMENT(t) ((t) ~ (t) 0 == (t) -1)
-# define TYPE_ONES_COMPLEMENT(t) ((t) ~ (t) 0 == 0)
-# define TYPE_SIGNED_MAGNITUDE(t) ((t) ~ (t) 0 < (t) -1)
+#define TYPE_TWOS_COMPLEMENT(t) ((t) ~ (t) 0 == (t) -1)
+#define TYPE_ONES_COMPLEMENT(t) ((t) ~ (t) 0 == 0)
+#define TYPE_SIGNED_MAGNITUDE(t) ((t) ~ (t) 0 < (t) -1)
+
+/* True if the signed integer expression E uses two's complement.  */
+#define _GL_INT_TWOS_COMPLEMENT(e) (~ _GL_INT_CONVERT (e, 0) == -1)
 
 /* True if the arithmetic type T is signed.  */
-# define TYPE_SIGNED(t) (! ((t) 0 < (t) -1))
-
-/* The maximum and minimum values for the integer type T.  These
+#define TYPE_SIGNED(t) (! ((t) 0 < (t) -1))
+
+/* Return 1 if the integer expression E, after integer promotion, has
+   a signed type.  E should not have side effects.  */
+#define _GL_INT_SIGNED(e) (_GL_INT_CONVERT (e, -1) < 0)
+
+
+/* Minimum and maximum values for integer types and expressions.  These
    macros have undefined behavior if T is signed and has padding bits.
    If this is a problem for you, please let us know how to fix it for
    your host.  */
-# define TYPE_MINIMUM(t) \
-  ((t) (! TYPE_SIGNED (t) \
-        ? (t) 0 \
-        : TYPE_SIGNED_MAGNITUDE (t) \
-        ? ~ (t) 0 \
+
+/* The maximum and minimum values for the integer type T.  */
+#define TYPE_MINIMUM(t)                                                 \
+  ((t) (! TYPE_SIGNED (t)                                               \
+        ? (t) 0                                                         \
+        : TYPE_SIGNED_MAGNITUDE (t)                                     \
+        ? ~ (t) 0                                                       \
         : ~ TYPE_MAXIMUM (t)))
-# define TYPE_MAXIMUM(t) \
-  ((t) (! TYPE_SIGNED (t) \
-        ? (t) -1 \
+#define TYPE_MAXIMUM(t)                                                 \
+  ((t) (! TYPE_SIGNED (t)                                               \
+        ? (t) -1                                                        \
         : ((((t) 1 << (sizeof (t) * CHAR_BIT - 2)) - 1) * 2 + 1)))
 
-/* Return zero if T can be determined to be an unsigned type.
-   Otherwise, return 1.
-   When compiling with GCC, INT_STRLEN_BOUND uses this macro to obtain a
-   tighter bound.  Otherwise, it overestimates the true bound by one byte
-   when applied to unsigned types of size 2, 4, 16, ... bytes.
-   The symbol signed_type_or_expr__ is private to this header file.  */
-# if __GNUC__ >= 2
-#  define signed_type_or_expr__(t) TYPE_SIGNED (__typeof__ (t))
-# else
-#  define signed_type_or_expr__(t) 1
-# endif
+/* The maximum and minimum values for the type of the expression E,
+   after integer promotion.  E should not have side effects.  */
+#define _GL_INT_MINIMUM(e)                                              \
+  (_GL_INT_SIGNED (e)                                                   \
+   ? - _GL_INT_TWOS_COMPLEMENT (e) - _GL_SIGNED_INT_MAXIMUM (e)         \
+   : _GL_INT_CONVERT (e, 0))
+#define _GL_INT_MAXIMUM(e)                                              \
+  (_GL_INT_SIGNED (e)                                                   \
+   ? _GL_SIGNED_INT_MAXIMUM (e)                                         \
+   : _GL_INT_CONVERT (e, -1))
+#define _GL_SIGNED_INT_MAXIMUM(e)                                       \
+  (((_GL_INT_CONVERT (e, 1) << (sizeof ((e) + 0) * CHAR_BIT - 2)) - 1) * 2 + 1)
+
+
+/* Return 1 if the __typeof__ keyword works.  This could be done by
+   'configure', but for now it's easier to do it by hand.  */
+#if 2 <= __GNUC__ || 0x5110 <= __SUNPRO_C
+# define _GL_HAVE___TYPEOF__ 1
+#else
+# define _GL_HAVE___TYPEOF__ 0
+#endif
+
+/* Return 1 if the integer type or expression T might be signed.  Return 0
+   if it is definitely unsigned.  This macro does not evaluate its argument,
+   and expands to an integer constant expression.  */
+#if _GL_HAVE___TYPEOF__
+# define _GL_SIGNED_TYPE_OR_EXPR(t) TYPE_SIGNED (__typeof__ (t))
+#else
+# define _GL_SIGNED_TYPE_OR_EXPR(t) 1
+#endif
 
 /* Bound on length of the string representing an unsigned integer
    value representable in B bits.  log10 (2.0) < 146/485.  The
    smallest value of B where this bound is not tight is 2621.  */
-# define INT_BITS_STRLEN_BOUND(b) (((b) * 146 + 484) / 485)
+#define INT_BITS_STRLEN_BOUND(b) (((b) * 146 + 484) / 485)
 
 /* Bound on length of the string representing an integer type or expression T.
    Subtract 1 for the sign bit if T is signed, and then add 1 more for
-   a minus sign if needed.  */
-# define INT_STRLEN_BOUND(t) \
-  (INT_BITS_STRLEN_BOUND (sizeof (t) * CHAR_BIT - signed_type_or_expr__ (t)) \
-   + signed_type_or_expr__ (t))
+   a minus sign if needed.
+
+   Because _GL_SIGNED_TYPE_OR_EXPR sometimes returns 0 when its argument is
+   signed, this macro may overestimate the true bound by one byte when
+   applied to unsigned types of size 2, 4, 16, ... bytes.  */
+#define INT_STRLEN_BOUND(t)                                     \
+  (INT_BITS_STRLEN_BOUND (sizeof (t) * CHAR_BIT                 \
+                          - _GL_SIGNED_TYPE_OR_EXPR (t))        \
+   + _GL_SIGNED_TYPE_OR_EXPR (t))
 
 /* Bound on buffer size needed to represent an integer type or expression T,
    including the terminating null.  */
-# define INT_BUFSIZE_BOUND(t) (INT_STRLEN_BOUND (t) + 1)
-
-#endif /* GL_INTPROPS_H */
+#define INT_BUFSIZE_BOUND(t) (INT_STRLEN_BOUND (t) + 1)
+
+
+/* Range overflow checks.
+
+   The INT_<op>_RANGE_OVERFLOW macros return 1 if the corresponding C
+   operators might not yield numerically correct answers due to
+   arithmetic overflow.  They do not rely on undefined or
+   implementation-defined behavior.  Their implementations are simple
+   and straightforward, but they are a bit harder to use than the
+   INT_<op>_OVERFLOW macros described below.
+
+   Example usage:
+
+     long int i = ...;
+     long int j = ...;
+     if (INT_MULTIPLY_RANGE_OVERFLOW (i, j, LONG_MIN, LONG_MAX))
+       printf ("multiply would overflow");
+     else
+       printf ("product is %ld", i * j);
+
+   Restrictions on *_RANGE_OVERFLOW macros:
+
+   These macros do not check for all possible numerical problems or
+   undefined or unspecified behavior: they do not check for division
+   by zero, for bad shift counts, or for shifting negative numbers.
+
+   These macros may evaluate their arguments zero or multiple times,
+   so the arguments should not have side effects.  The arithmetic
+   arguments (including the MIN and MAX arguments) must be of the same
+   integer type after the usual arithmetic conversions, and the type
+   must have minimum value MIN and maximum MAX.  Unsigned types should
+   use a zero MIN of the proper type.
+
+   These macros are tuned for constant MIN and MAX.  For commutative
+   operations such as A + B, they are also tuned for constant B.  */
+
+/* Return 1 if A + B would overflow in [MIN,MAX] arithmetic.
+   See above for restrictions.  */
+#define INT_ADD_RANGE_OVERFLOW(a, b, min, max)          \
+  ((b) < 0                                              \
+   ? (a) < (min) - (b)                                  \
+   : (max) - (b) < (a))
+
+/* Return 1 if A - B would overflow in [MIN,MAX] arithmetic.
+   See above for restrictions.  */
+#define INT_SUBTRACT_RANGE_OVERFLOW(a, b, min, max)     \
+  ((b) < 0                                              \
+   ? (max) + (b) < (a)                                  \
+   : (a) < (min) + (b))
+
+/* Return 1 if - A would overflow in [MIN,MAX] arithmetic.
+   See above for restrictions.  */
+#define INT_NEGATE_RANGE_OVERFLOW(a, min, max)          \
+  ((min) < 0                                            \
+   ? (a) < - (max)                                      \
+   : 0 < (a))
+
+/* Return 1 if A * B would overflow in [MIN,MAX] arithmetic.
+   See above for restrictions.  */
+#define INT_MULTIPLY_RANGE_OVERFLOW(a, b, min, max)     \
+  ((b) < 0                                              \
+   ? ((a) < 0                                           \
+      ? (a) < (max) / (b)                               \
+      : (b) < -1 && (min) / (b) < (a))                  \
+   : (0 < (b)                                           \
+      && ((a) < 0                                       \
+          ? (a) < (min) / (b)                           \
+          : (max) / (b) < (a))))
+
+/* Return 1 if A / B would overflow in [MIN,MAX] arithmetic.
+   See above for restrictions.  Do not check for division by zero.  */
+#define INT_DIVIDE_RANGE_OVERFLOW(a, b, min, max)       \
+  ((min) < 0 && (b) == -1 && (a) < - (max))
+
+/* Return 1 if A % B would overflow in [MIN,MAX] arithmetic.
+   See above for restrictions.  Do not check for division by zero.
+   Mathematically, % should never overflow, but on x86-like hosts
+   INT_MIN % -1 traps, and the C standard permits this, so treat this
+   as an overflow too.  */
+#define INT_REMAINDER_RANGE_OVERFLOW(a, b, min, max)    \
+  INT_DIVIDE_RANGE_OVERFLOW (a, b, min, max)
+
+/* Return 1 if A << B would overflow in [MIN,MAX] arithmetic.
+   See above for restrictions.  Here, MIN and MAX are for A only, and B need
+   not be of the same type as the other arguments.  The C standard says that
+   behavior is undefined for shifts unless 0 <= B < wordwidth, and that when
+   A is negative then A << B has undefined behavior and A >> B has
+   implementation-defined behavior, but do not check these other
+   restrictions.  */
+#define INT_LEFT_SHIFT_RANGE_OVERFLOW(a, b, min, max)   \
+  ((a) < 0                                              \
+   ? (a) < (min) >> (b)                                 \
+   : (max) >> (b) < (a))
+
+
+/* The _GL*_OVERFLOW macros have the same restrictions as the
+   *_RANGE_OVERFLOW macros, except that they do not assume that operands
+   (e.g., A and B) have the same type as MIN and MAX.  Instead, they assume
+   that the result (e.g., A + B) has that type.  */
+#define _GL_ADD_OVERFLOW(a, b, min, max)                                \
+  ((min) < 0 ? INT_ADD_RANGE_OVERFLOW (a, b, min, max)                  \
+   : (a) < 0 ? (b) <= (a) + (b)                                         \
+   : (b) < 0 ? (a) <= (a) + (b)                                         \
+   : (a) + (b) < (b))
+#define _GL_SUBTRACT_OVERFLOW(a, b, min, max)                           \
+  ((min) < 0 ? INT_SUBTRACT_RANGE_OVERFLOW (a, b, min, max)             \
+   : (a) < 0 ? 1                                                        \
+   : (b) < 0 ? (a) - (b) <= (a)                                         \
+   : (a) < (b))
+#define _GL_MULTIPLY_OVERFLOW(a, b, min, max)                           \
+  (((min) == 0 && (((a) < 0 && 0 < (b)) || ((b) < 0 && 0 < (a))))       \
+   || INT_MULTIPLY_RANGE_OVERFLOW (a, b, min, max))
+#define _GL_DIVIDE_OVERFLOW(a, b, min, max)                             \
+  ((min) < 0 ? (b) == _GL_INT_CONVERT (min, -1) && (a) < - (max)        \
+   : (a) < 0 ? (b) <= (a) + (b) - 1                                     \
+   : (b) < 0 && (a) + (b) <= (a))
+#define _GL_REMAINDER_OVERFLOW(a, b, min, max)                          \
+  ((min) < 0 ? (b) == _GL_INT_CONVERT (min, -1) && (a) < - (max)        \
+   : (a) < 0 ? (a) % (b) != ((max) - (b) + 1) % (b)                     \
+   : (b) < 0 && ! _GL_UNSIGNED_NEG_MULTIPLE (a, b, max))
+
+/* Return a nonzero value if A is a mathematical multiple of B, where
+   A is unsigned, B is negative, and MAX is the maximum value of A's
+   type.  A's type must be the same as (A % B)'s type.  Normally (A %
+   -B == 0) suffices, but things get tricky if -B would overflow.  */
+#define _GL_UNSIGNED_NEG_MULTIPLE(a, b, max)                            \
+  (((b) < -_GL_SIGNED_INT_MAXIMUM (b)                                   \
+    ? (_GL_SIGNED_INT_MAXIMUM (b) == (max)                              \
+       ? (a)                                                            \
+       : (a) % (_GL_INT_CONVERT (a, _GL_SIGNED_INT_MAXIMUM (b)) + 1))   \
+    : (a) % - (b))                                                      \
+   == 0)
+
+
+/* Integer overflow checks.
+
+   The INT_<op>_OVERFLOW macros return 1 if the corresponding C operators
+   might not yield numerically correct answers due to arithmetic overflow.
+   They work correctly on all known practical hosts, and do not rely
+   on undefined behavior due to signed arithmetic overflow.
+
+   Example usage:
+
+     long int i = ...;
+     long int j = ...;
+     if (INT_MULTIPLY_OVERFLOW (i, j))
+       printf ("multiply would overflow");
+     else
+       printf ("product is %ld", i * j);
+
+   These macros do not check for all possible numerical problems or
+   undefined or unspecified behavior: they do not check for division
+   by zero, for bad shift counts, or for shifting negative numbers.
+
+   These macros may evaluate their arguments zero or multiple times, so the
+   arguments should not have side effects.
+
+   These macros are tuned for their last argument being a constant.
+
+   Return 1 if the integer expressions A * B, A - B, -A, A * B, A / B,
+   A % B, and A << B would overflow, respectively.  */
+
+#define INT_ADD_OVERFLOW(a, b) \
+  _GL_BINARY_OP_OVERFLOW (a, b, _GL_ADD_OVERFLOW)
+#define INT_SUBTRACT_OVERFLOW(a, b) \
+  _GL_BINARY_OP_OVERFLOW (a, b, _GL_SUBTRACT_OVERFLOW)
+#define INT_NEGATE_OVERFLOW(a) \
+  INT_NEGATE_RANGE_OVERFLOW (a, _GL_INT_MINIMUM (a), _GL_INT_MAXIMUM (a))
+#define INT_MULTIPLY_OVERFLOW(a, b) \
+  _GL_BINARY_OP_OVERFLOW (a, b, _GL_MULTIPLY_OVERFLOW)
+#define INT_DIVIDE_OVERFLOW(a, b) \
+  _GL_BINARY_OP_OVERFLOW (a, b, _GL_DIVIDE_OVERFLOW)
+#define INT_REMAINDER_OVERFLOW(a, b) \
+  _GL_BINARY_OP_OVERFLOW (a, b, _GL_REMAINDER_OVERFLOW)
+#define INT_LEFT_SHIFT_OVERFLOW(a, b) \
+  INT_LEFT_SHIFT_RANGE_OVERFLOW (a, b, \
+                                 _GL_INT_MINIMUM (a), _GL_INT_MAXIMUM (a))
+
+/* Return 1 if the expression A <op> B would overflow,
+   where OP_RESULT_OVERFLOW (A, B, MIN, MAX) does the actual test,
+   assuming MIN and MAX are the minimum and maximum for the result type.
+
+   This macro assumes that A | B is a valid integer if both A and B are,
+   which is true of all known practical hosts.  If this is a problem
+   for you, please let us know how to fix it for your host.  */
+#define _GL_BINARY_OP_OVERFLOW(a, b, op_result_overflow)        \
+  op_result_overflow (a, b,                                     \
+                      _GL_INT_MINIMUM ((a) | (b)),              \
+                      _GL_INT_MAXIMUM ((a) | (b)))
+
+#endif /* _GL_INTPROPS_H */

=== modified file 'lib/unistd.in.h'
--- lib/unistd.in.h	2011-04-18 04:03:18 +0000
+++ lib/unistd.in.h	2011-05-16 00:51:54 +0000
@@ -871,6 +871,22 @@
 #endif
 
 
+#if @GNULIB_GROUP_MEMBER@
+/* Determine whether group id is in calling user's group list.  */
+# if !@HAVE_GROUP_MEMBER@
+_GL_FUNCDECL_SYS (group_member, int, (gid_t gid));
+# endif
+_GL_CXXALIAS_SYS (group_member, int, (gid_t gid));
+_GL_CXXALIASWARN (group_member);
+#elif defined GNULIB_POSIXCHECK
+# undef group_member
+# if HAVE_RAW_DECL_GROUP_MEMBER
+_GL_WARN_ON_USE (group_member, "group_member is unportable - "
+                 "use gnulib module group-member for portability");
+# endif
+#endif
+
+
 #if @GNULIB_LCHOWN@
 /* Change the owner of FILE to UID (if UID is not -1) and the group of FILE
    to GID (if GID is not -1).  Do not follow symbolic links.

=== modified file 'm4/unistd_h.m4'
--- m4/unistd_h.m4	2011-04-18 04:03:18 +0000
+++ m4/unistd_h.m4	2011-05-16 00:51:54 +0000
@@ -1,4 +1,4 @@
-# unistd_h.m4 serial 55
+# unistd_h.m4 serial 56
 dnl Copyright (C) 2006-2011 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -36,8 +36,8 @@
     ]], [chown dup2 dup3 environ euidaccess faccessat fchdir fchownat
     fsync ftruncate getcwd getdomainname getdtablesize getgroups
     gethostname getlogin getlogin_r getpagesize getusershell setusershell
-    endusershell lchown link linkat lseek pipe pipe2 pread pwrite readlink
-    readlinkat rmdir sleep symlink symlinkat ttyname_r unlink unlinkat
+    endusershell group_member lchown link linkat lseek pipe pipe2 pread pwrite
+    readlink readlinkat rmdir sleep symlink symlinkat ttyname_r unlink unlinkat
     usleep])
 ])
 
@@ -72,6 +72,7 @@
   GNULIB_GETLOGIN_R=0;           AC_SUBST([GNULIB_GETLOGIN_R])
   GNULIB_GETPAGESIZE=0;          AC_SUBST([GNULIB_GETPAGESIZE])
   GNULIB_GETUSERSHELL=0;         AC_SUBST([GNULIB_GETUSERSHELL])
+  GNULIB_GROUP_MEMBER=0;         AC_SUBST([GNULIB_GROUP_MEMBER])
   GNULIB_LCHOWN=0;               AC_SUBST([GNULIB_LCHOWN])
   GNULIB_LINK=0;                 AC_SUBST([GNULIB_LINK])
   GNULIB_LINKAT=0;               AC_SUBST([GNULIB_LINKAT])
@@ -110,6 +111,7 @@
   HAVE_GETHOSTNAME=1;     AC_SUBST([HAVE_GETHOSTNAME])
   HAVE_GETLOGIN=1;        AC_SUBST([HAVE_GETLOGIN])
   HAVE_GETPAGESIZE=1;     AC_SUBST([HAVE_GETPAGESIZE])
+  HAVE_GROUP_MEMBER=1;    AC_SUBST([HAVE_GROUP_MEMBER])
   HAVE_LCHOWN=1;          AC_SUBST([HAVE_LCHOWN])
   HAVE_LINK=1;            AC_SUBST([HAVE_LINK])
   HAVE_LINKAT=1;          AC_SUBST([HAVE_LINKAT])

bug#8675: lisp_string_width and strings wider than INT_MAX

[Eli Zaretskii]

> Date: Sun, 15 May 2011 22:07:36 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> PATCH 3 depends on two obvious patches: PATCH 2 introduces a helper
> no-return function string_overflow, and PATCH 1 updates to the latest
> version of gnulib.

Thanks, but why do these patches come with unrelated changes in
texinfo.tex?

bug#8675: lisp_string_width and strings wider than INT_MAX

[Paul Eggert]

On 05/15/11 22:30, Eli Zaretskii wrote:
>> Date: Sun, 15 May 2011 22:07:36 -0700
>> From: Paul Eggert <eggert@cs.ucla.edu>
>>
>> PATCH 3 depends on two obvious patches: PATCH 2 introduces a helper
>> no-return function string_overflow, and PATCH 1 updates to the latest
>> version of gnulib.
> 
> Thanks, but why do these patches come with unrelated changes in
> texinfo.tex?

Because that's part of PATCH 1, which updates to the latest version
of gnulib.  Gnulib contains texinfo.tex.  PATCH 1 was generated
entirely automatically by "make sync-from-gnulib".

bug#8675: lisp_string_width and strings wider than INT_MAX

[Eli Zaretskii]

> Date: Sun, 15 May 2011 22:33:03 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 8675@debbugs.gnu.org
> 
> On 05/15/11 22:30, Eli Zaretskii wrote:
> >> Date: Sun, 15 May 2011 22:07:36 -0700
> >> From: Paul Eggert <eggert@cs.ucla.edu>
> >>
> >> PATCH 3 depends on two obvious patches: PATCH 2 introduces a helper
> >> no-return function string_overflow, and PATCH 1 updates to the latest
> >> version of gnulib.
> > 
> > Thanks, but why do these patches come with unrelated changes in
> > texinfo.tex?
> 
> Because that's part of PATCH 1, which updates to the latest version
> of gnulib.  Gnulib contains texinfo.tex.  PATCH 1 was generated
> entirely automatically by "make sync-from-gnulib".

When you merge, could you please make the texinfo.tex update a
separate commit on the trunk, then?  No one will ever expect to find
that file in a commit logged as "sync from gnulib", which will make
forensics more difficult than it needs to (since your commits are
always merge-commits).

TIA

bug#8675: lisp_string_width and strings wider than INT_MAX

[Paul Eggert]

On 05/16/11 00:57, Eli Zaretskii wrote:
> When you merge, could you please make the texinfo.tex update a
> separate commit on the trunk, then?  No one will ever expect to find
> that file in a commit logged as "sync from gnulib", which will make
> forensics more difficult than it needs to (since your commits are
> always merge-commits).

Currently what I do is "make sync-from-gnulib" followed by
"bzr commit".  This is requesting that I go to more work, by
breaking up the merge from gnulib into smaller functional pieces,
one for each functional change in gnulib, and installing them
individually.

I'd rather not.  Not only would this be more work, it would
run contrary to other advice I've gotten, which is to batch
changes instead of installing lots of little changes one at a time.
Besides, I doubt whether including texinfo.tex confuses forensics much,
so it's not at all clear that the extra work would be an overall win.

Gnulib changes are like autogen changes.  They're mostly ignorable,
but if a problem comes up, one can do forensics by going to the
repository we're importing from and looking at its history.
When autotools change, we don't commit the resulting changes to
"configure" individually, and that's a similar situation.

bug#8675: lisp_string_width and strings wider than INT_MAX

[Stefan Monnier]

> When you merge, could you please make the texinfo.tex update a
> separate commit on the trunk, then?

Why?

> No one will ever expect to find that file in a commit logged as "sync
> from gnulib",

I, for one, do.  texinfo.tex is not a file we maintain, so I fully
expect to find its changes in some kind of "sync from outside" commit.


        Stefan

bug#8675: lisp_string_width and strings wider than INT_MAX

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Paul Eggert <eggert@cs.ucla.edu>,  8675@debbugs.gnu.org
> Date: Mon, 16 May 2011 13:48:33 -0300
> 
> > When you merge, could you please make the texinfo.tex update a
> > separate commit on the trunk, then?
> 
> Why?
> 
> > No one will ever expect to find that file in a commit logged as "sync
> > from gnulib",
> 
> I, for one, do.  texinfo.tex is not a file we maintain, so I fully
> expect to find its changes in some kind of "sync from outside" commit.

I'm moving this sub-thread to emacs-devel, as it is no longer a casual
remark related to this bug.

bug#8675: committed fix into trunk

[Paul Eggert]

Bzr 104265,which I just committed into the trunk,
should contain the fix discussed above, so I'm
marking this as "done".  As requested I separated
the gnulib merge into a separate commit, bzr 104264.

bug#8675: committed fix into trunk

[Christoph Scholtes]

On 5/17/2011 7:33 PM, Paul Eggert wrote:
> Bzr 104265,which I just committed into the trunk,
> should contain the fix discussed above, so I'm
> marking this as "done".  As requested I separated
> the gnulib merge into a separate commit, bzr 104264.

bzr 104264 breaks the current build on GNU/Linux (Debian Squeeze with 
gcc - Debian 4.4.5-8):

gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../src 
-I/home/devel/emacs/trunk/src     -g -O2 -MT careadlinkat.o -MD -MP -MF 
.deps/careadlinkat.Tpo -c -o careadlinkat.o careadlinkat.c
In file included from careadlinkat.h:24,
                  from careadlinkat.c:23:
./unistd.h:1186:5: error: token "@" is not valid in preprocessor expressions
make[3]: *** [careadlinkat.o] Error 1
make[3]: Leaving directory `/home/devel/emacs/trunk/lib'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/home/devel/emacs/trunk/lib'
make[1]: *** [lib] Error 2
make[1]: Leaving directory `/home/devel/emacs/trunk'
make: *** [bootstrap] Error 2

 From unistd.h, l.1186:

#if @GNULIB_GROUP_MEMBER@
/* Determine whether group id is in calling user's group list.  */
# if !@HAVE_GROUP_MEMBER@
_GL_FUNCDECL_SYS (group_member, int, (gid_t gid));
# endif


Note, that there are other instances with @ in the file.

Christoph

bug#8675: committed fix into trunk

[Paul Eggert]

On 05/17/11 19:26, Christoph Scholtes wrote:
> ./unistd.h:1186:5: error: token "@" is not valid in preprocessor expressions

I can't reproduce the problem on Ubuntu 11.04 (stock gcc 4.5.2-8ubuntu4)
with bzr 104265.

That diagnostic is a symptom of doing a "bzr up" over a (partly?) built
old checkout.  I suggest trying "./autogen.sh; ./configure; make clean; make"
or you can simply start with a fresh checkout of the trunk.

bug#8675: committed fix into trunk

[Christoph Scholtes]

On 5/17/2011 8:48 PM, Paul Eggert wrote:
> On 05/17/11 19:26, Christoph Scholtes wrote:
>> ./unistd.h:1186:5: error: token "@" is not valid in preprocessor expressions
>
> I can't reproduce the problem on Ubuntu 11.04 (stock gcc 4.5.2-8ubuntu4)
> with bzr 104265.
>
> That diagnostic is a symptom of doing a "bzr up" over a (partly?) built
> old checkout.  I suggest trying "./autogen.sh; ./configure; make clean; make"
> or you can simply start with a fresh checkout of the trunk.

The ./autogen.sh seemed to have fixed the issue. Sorry for the noise.

bug#8664: committed fix into trunk

[Eli Zaretskii]

> Date: Tue, 17 May 2011 18:33:25 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> As requested I separated the gnulib merge into a separate commit,
> bzr 104264.

Thanks.  You forgot ChangeLog entries for the files imported from
gnulib.  I fixed that.

bug#8675: committed fix into trunk

[Andreas Schwab]

Paul Eggert <eggert@cs.ucla.edu> writes:

> That diagnostic is a symptom of doing a "bzr up" over a (partly?) built
> old checkout.

If that does not work, then proper dependencies are missing.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#8675: error: token "@" is not valid in preprocessor expressions

[Paul Eggert]

[Adding bug-gnulib to this thread.  For bug-gnulib readers, the scenario is
 in Emacs a "bzr update; make" failed with:

  ./unistd.h:1186:5: error: token "@" is not valid in preprocessor expressions

 because unistd.h was built with the old Makefile.
 Full thread at <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8675#31>.
]

On 05/18/11 05:39, Andreas Schwab wrote:
> If that does not work, then proper dependencies are missing.

I don't see how adding a dependency would fix this problem.  In this
case, lib/Makefile in turn depended on 'configure', 'configure.in',
'm4/longlong.m4', etc., etc., and one of these files got updated, so
lib/Makefile was regenerated; but as I understand it, the 'make' that was
(still) running was based on the out-of-date lib/Makefile, and it
generated the a bad lib/unistd.h.

Adding a dependency "unistd.h: Makefile" wouldn't fix this problem.

This sort of problem is not due to gnulib per se; it's inherent to any
project that uses makefiles.  In general, if a patch modifies a
makefile, or anything the makefile depends on, then you must
regenerate everything from scratch with a fresh 'make' invocation.

That being said, I've run into Christoph's problem myself more than
once, and it's a hassle, and it'd be nice to address it somehow.  How
about this idea?  I expect it would have worked around this problem.
Currently lib/Makefile contains something like this:

	unistd.h: <dependencies>
		<big-hairy-command>

Suppose we change this rule to look like this:

	unistd.h: Makefile <dependencies>
		case ' $? ,$(USING_NEW_MAKEFILE)' in \
		  *' Makefile '*,) \
		    exec $(MAKE) $(AM_MAKEFLAGS) USING_NEW_MAKEFILE=yes $@;; \
		esac; \
		<big-hairy-command>

This would be a gnulib change, so I'll CC: this to bug-gnulib.
A similar pattern would apply to every module that generates
a .h file with a big hairy command that uses 'make' variables.

bug#8675: error: token "@" is not valid in preprocessor expressions

[Andreas Schwab]

Paul Eggert <eggert@cs.ucla.edu> writes:

> I don't see how adding a dependency would fix this problem.  In this
> case, lib/Makefile in turn depended on 'configure', 'configure.in',
> 'm4/longlong.m4', etc., etc., and one of these files got updated, so
> lib/Makefile was regenerated; but as I understand it, the 'make' that was
> (still) running was based on the out-of-date lib/Makefile, and it
> generated the a bad lib/unistd.h.

Looks like an orderring problem.  Normally, if GNU make sees that a
makefile is remade it rereads it automatically.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#8675: error: token "@" is not valid in preprocessor expressions

[Paul Eggert]

On 05/18/11 13:35, Andreas Schwab wrote:
> Normally, if GNU make sees that a
> makefile is remade it rereads it automatically.

Ah, thanks, I didn't know that.  So if we care only about
GNU make, then all we need to do is to have unistd.h
depend on Makefile.  And, once unistd.h depends on Makefile
then it need not depend on config.status (as Makefile already
depends on config.status).  Like this:

--- a/modules/unistd
+++ b/modules/unistd
@@ -20,7 +20,7 @@ BUILT_SOURCES += unistd.h
 
 # We need the following in order to create an empty placeholder for
 # <unistd.h> when the system doesn't have one.
-unistd.h: unistd.in.h $(top_builddir)/config.status $(CXXDEFS_H) $(ARG_NONNULL_H) $(WARN_ON_USE_H)
+unistd.h: unistd.in.h Makefile $(CXXDEFS_H) $(ARG_NONNULL_H) $(WARN_ON_USE_H)
 	$(AM_V_GEN)rm -f $@-t $@ && \
 	{ echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
 	  sed -e 's|@''HAVE_UNISTD_H''@|$(HAVE_UNISTD_H)|g' \


Looking at the existing gnulib modules, I see that almost all of them
have the .h file depend on config.status, but there's one exception:
configmake.h depends on Makefile.  Shouldn't they all depend on
Makefile rather than on config.status?  That should have helped to
avoid this problem.

bug#8675: error: token "@" is not valid in preprocessor expressions

[Bruno Haible]

Hi Paul,

> all we need to do is to have unistd.h
> depend on Makefile.  And, once unistd.h depends on Makefile
> then it need not depend on config.status (as Makefile already
> depends on config.status).  Like this:
> 
> --- a/modules/unistd
> +++ b/modules/unistd
> @@ -20,7 +20,7 @@ BUILT_SOURCES += unistd.h
>  
>  # We need the following in order to create an empty placeholder for
>  # <unistd.h> when the system doesn't have one.
> -unistd.h: unistd.in.h $(top_builddir)/config.status $(CXXDEFS_H) $(ARG_NONNULL_H) $(WARN_ON_USE_H)
> +unistd.h: unistd.in.h Makefile $(CXXDEFS_H) $(ARG_NONNULL_H) $(WARN_ON_USE_H)
>  	$(AM_V_GEN)rm -f $@-t $@ && \
>  	{ echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
>  	  sed -e 's|@''HAVE_UNISTD_H''@|$(HAVE_UNISTD_H)|g' \
> 

Such a change would have the effect that hand-editing a Makefile would
cause all gnulib *.h files to be regenerated and, with it, all *.o files
would be recompiled. However, hand-editing a Makefile is necessary in
two situations at least:
  1) to test a certain modification of compilation commands, before
     putting the changes into Makefile.am,
  2) to trace the effect of C compiler bugs, by compiling different
     *.o files with different compiler optimization levels.

I'm therefore vehemently opposed to such a change.

In fact, in the thread starting at
  <http://lists.gnu.org/archive/html/bug-gnulib/2011-04/msg00013.html>
we developed some Automake tests for guaranteeing that everything is properly
rebuilt, and it turned out that
  1. some specific code pattern is necessary in the module descriptions, and
     it was implemented on 2011-04-03,
  2. GNU make is necessary as well.
But that's it.

So, if the reporter was using GNU make
    and the previous Makefile.in was based on gnulib 2011-04-03 or newer
    and the reporter did a make command in the top-level directory that
        recreated config.status before recursing into lib/ and then into
        src/,
then the situation cannot have occurred.

If the reporter was not using GNU make, he should do so. It has become clear
through the tests in Automake that GNU make is a requirement for rebuilds to
be reliable.

If the previous Makefile.in was not based on gnulib 2011-04-03 or newer,
we need to do nothing; the problem is already fixed.

If the reporter did "make" in the top-level directory of emacs and it did
not rebuild config.status, even after configure changed, it needs to be
fixed in emacs.

If the reporter only did "make" in the lib/ or src/ subdirectory and not
in the top-level directory, then either he needs to change his way of working,
or a rule like

  ../config.status : $(srcdir)/../configure
  	../config.status --recheck

needs to be added in every subdirectory's Makefile. Automake generated
Makefiles contain such a rule, so maybe that's what is missing in
emacs/src/Makefile?

Bruno
-- 
In memoriam Eli Cohen <http://en.wikipedia.org/wiki/Eli_Cohen>

bug#8675: error: token "@" is not valid in preprocessor expressions

[Christoph Scholtes]

On 5/18/2011 6:27 PM, Bruno Haible wrote:

> So, if the reporter was using GNU make
>      and the previous Makefile.in was based on gnulib 2011-04-03 or newer
>      and the reporter did a make command in the top-level directory that
>          recreated config.status before recursing into lib/ and then into
>          src/,
> then the situation cannot have occurred.

I was using Debian Squeeze with GNU make 3.81. I executed `make 
maintainer-clean' at the root and then `./configure' and `make 
bootstrap' when it failed.

> If the previous Makefile.in was not based on gnulib 2011-04-03 or newer,
> we need to do nothing; the problem is already fixed.

I am not sure what version of gnulib my Makefile was based on.

> If the reporter did "make" in the top-level directory of emacs and it did
> not rebuild config.status, even after configure changed, it needs to be
> fixed in emacs.

I am not sure how to determine if this was the case, since running 
./autogen.sh, ./configure and make bootstrap fixed the problem.

Let me know if I can help to further troubleshoot this.

Christoph

bug#8675: error: token "@" is not valid in preprocessor expressions

[Paul Eggert]

>> If the previous Makefile.in was not based on gnulib 2011-04-03 or newer,
>> we need to do nothing; the problem is already fixed.
> 
> I am not sure what version of gnulib my Makefile was based on.

The previous Emacs trunk commit (i.e., the commit before the one that
caused you a problem) was based on gnulib 2011-05-06.  I
also updated Emacs trunk from gnulib on 2011-04-26, 2011-04-17,
2011-04-10, 2011-04-06, and 2011-04-05.  So if
you've been keeping up to date regularly with Emacs, your
old Makefile was based on a suitably-recent gnulib.

>> If the reporter did "make" in the top-level directory of emacs and it did
>> not rebuild config.status, even after configure changed, it needs to be
>> fixed in emacs.
> 
> I am not sure how to determine if this was the case, since running
> ./autogen.sh, ./configure and make bootstrap fixed the problem.
> 
> Let me know if I can help to further troubleshoot this.

Perhaps you could try checking out the old version, building it, and
then doing a "bzr up", and see whether the problem recurs?

The top-level Emacs directory does have a dependency of config.status
on configure, for what that's worth, so a top-level "make" should
rebuild config.status.

> If the reporter only did "make" in the lib/ or src/ subdirectory and not
> in the top-level directory, then either he needs to change his way of working,
> or a rule like
> 
>   ../config.status : $(srcdir)/../configure
>   	../config.status --recheck
> 
> needs to be added in every subdirectory's Makefile. Automake generated
> Makefiles contain such a rule, so maybe that's what is missing in
> emacs/src/Makefile?

His problem occurred in emacs/lib, and emacs/lib/Makefile is
generated by Automake, so I expect this is not the problem.