GNU bug report logs - #8675
lisp_string_width and strings wider than INT_MAX

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8675 in the body.
You can then email your comments to 8675 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: lisp_string_width and strings wider than INT_MAX
Date: Sun, 15 May 2011 22:07:36 -0700

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 <at> 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 <at> 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 <at> 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])

Message #8 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: lisp_string_width and strings wider than INT_MAX
Date: Mon, 16 May 2011 08:30:13 +0300

> Date: Sun, 15 May 2011 22:07:36 -0700
> From: Paul Eggert <eggert <at> 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?

Message #11 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: lisp_string_width and strings wider than INT_MAX
Date: Sun, 15 May 2011 22:33:03 -0700

On 05/15/11 22:30, Eli Zaretskii wrote:
>> Date: Sun, 15 May 2011 22:07:36 -0700
>> From: Paul Eggert <eggert <at> 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".

Message #14 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: lisp_string_width and strings wider than INT_MAX
Date: Mon, 16 May 2011 10:57:34 +0300

> Date: Sun, 15 May 2011 22:33:03 -0700
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> CC: 8675 <at> 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 <at> 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

Message #17 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: lisp_string_width and strings wider than INT_MAX
Date: Mon, 16 May 2011 09:37:07 -0700

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.

Message #20 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: lisp_string_width and strings wider than INT_MAX
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.


        Stefan

Message #23 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: eggert <at> cs.ucla.edu, 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: lisp_string_width and strings wider than INT_MAX
Date: Tue, 17 May 2011 05:52:42 -0400

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Paul Eggert <eggert <at> cs.ucla.edu>,  8675 <at> 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.

Message #28 received at 8675-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 8675-done <at> debbugs.gnu.org, 8664-done <at> debbugs.gnu.org
Subject: committed fix into trunk
Date: Tue, 17 May 2011 18:33:25 -0700

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.

Message #31 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Christoph Scholtes <cschol2112 <at> googlemail.com>
To: 8675 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#8675: committed fix into trunk
Date: Tue, 17 May 2011 20:26:04 -0600

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

Message #34 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Christoph Scholtes <cschol2112 <at> googlemail.com>
Cc: 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: committed fix into trunk
Date: Tue, 17 May 2011 19:48:50 -0700

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.

Message #37 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Christoph Scholtes <cschol2112 <at> googlemail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: committed fix into trunk
Date: Tue, 17 May 2011 21:19:28 -0600

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.

Message #40 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Christoph Scholtes <cschol2112 <at> googlemail.com>, 8675 <at> debbugs.gnu.org
Subject: Re: bug#8675: committed fix into trunk
Date: Wed, 18 May 2011 14:39:34 +0200

Paul Eggert <eggert <at> 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 <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #43 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Christoph Scholtes <cschol2112 <at> googlemail.com>,
	Bug-gnulib <bug-gnulib <at> gnu.org>, 8675 <at> debbugs.gnu.org
Subject: Re: error: token "@" is not valid in preprocessor expressions
Date: Wed, 18 May 2011 12:55:13 -0700

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

Message #46 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Christoph Scholtes <cschol2112 <at> googlemail.com>,
	Bug-gnulib <bug-gnulib <at> gnu.org>, 8675 <at> debbugs.gnu.org
Subject: Re: error: token "@" is not valid in preprocessor expressions
Date: Wed, 18 May 2011 22:35:54 +0200

Paul Eggert <eggert <at> 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 <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #49 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Christoph Scholtes <cschol2112 <at> googlemail.com>,
	Bug-gnulib <bug-gnulib <at> gnu.org>, 8675 <at> debbugs.gnu.org
Subject: Re: error: token "@" is not valid in preprocessor expressions
Date: Wed, 18 May 2011 15:59:14 -0700

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.

Message #52 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Haible <bruno <at> clisp.org>
To: bug-gnulib <at> gnu.org
Cc: Christoph Scholtes <cschol2112 <at> googlemail.com>, 8675 <at> debbugs.gnu.org,
	Paul Eggert <eggert <at> cs.ucla.edu>, Andreas Schwab <schwab <at> linux-m68k.org>
Subject: Re: error: token "@" is not valid in preprocessor expressions
Date: Thu, 19 May 2011 02:27:24 +0200

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>

Message #55 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Christoph Scholtes <cschol2112 <at> googlemail.com>
To: Bruno Haible <bruno <at> clisp.org>
Cc: Andreas Schwab <schwab <at> linux-m68k.org>, 8675 <at> debbugs.gnu.org,
	Paul Eggert <eggert <at> cs.ucla.edu>, bug-gnulib <at> gnu.org
Subject: Re: bug#8675: error: token "@" is not valid in preprocessor
	expressions
Date: Wed, 18 May 2011 19:47:43 -0600

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

Message #58 received at 8675 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Christoph Scholtes <cschol2112 <at> googlemail.com>
Cc: bug-gnulib <at> gnu.org, Andreas Schwab <schwab <at> linux-m68k.org>,
	8675 <at> debbugs.gnu.org, Bruno Haible <bruno <at> clisp.org>
Subject: Re: bug#8675: error: token "@" is not valid in preprocessor
	expressions
Date: Thu, 19 May 2011 00:39:47 -0700

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.