Fix doprnt so it could be used safely in `verror'. (Bug#8435)

 src/doprnt.c: Include limits.h.
 (SIZE_MAX): New macro.
 (doprnt): Return a size_t value.  2nd arg is now size_t.  Many
 local variables are now size_t instead of int or unsigned.
 Improve overflow protection.  Support `l' modifier for integer
 conversions.  Support %l conversion.  Don't assume an EMACS_INT
 argument for integer conversions and for %c.
 src/lisp.h (doprnt): Restore prototype.
 src/makefile.w32-in ($(BLD)/callint.$(O)): Depend on
 $(SRC)/character.h.
 src/Makefile.in (base_obj): Add back doprnt.o.
 src/deps.mk (doprnt.o): Add back prerequisites.
 (callint.o): Depend on character.h.
 src/eval.c (internal_lisp_condition_case): Include the handler
 representation in the error message.
 (verror): Call doprnt instead of vsnprintf.  Fix an off-by-one bug
 when breaking from the loop.
 src/xdisp.c (vmessage): Call doprnt instead of vsnprintf.
 src/callint.c (Fcall_interactively): When displaying error message
 about invalid control letter, pass the character's codepoint, not
 a pointer to its multibyte form.  Improve display of the character
 in octal and display also its hex code.
 src/character.c (char_string): Use %x to display the (unsigned)
 codepoint of an invalid character, to avoid displaying a bogus
 negative value.
 src/font.c (check_otf_features): Pass SDATA of SYMBOL_NAME to
 `error', not SYMBOL_NAME itself.
 src/coding.c (Fencode_sjis_char, Fencode_big5_char): Use %c for
 character arguments to `error'.
 src/charset.c (check_iso_charset_parameter): Fix incorrect argument
 to `error' in error message about FINAL_CHAR argument.  Make sure
 FINAL_CHAR is a character, and use %c when it is passed as
 argument to `error'.

13 files changed, 165 insertions, 83 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index 0067163edbf..831fb6afcd0 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,53 @@
 2011-04-23  Eli Zaretskii  <eliz@gnu.org>
 
+        Fix doprnt so it could be used again safely in `verror'.  (Bug#8435)
+        * doprnt.c: Include limits.h.
+        (SIZE_MAX): New macro.
+        (doprnt): Return a size_t value.  2nd arg is now size_t.  Many
+        local variables are now size_t instead of int or unsigned.
+        Improve overflow protection.  Support `l' modifier for integer
+        conversions.  Support %l conversion.  Don't assume an EMACS_INT
+        argument for integer conversions and for %c.
+
+        * lisp.h (doprnt): Restore prototype.
+
+        * makefile.w32-in ($(BLD)/callint.$(O)): Depend on
+        $(SRC)/character.h.
+
+        * Makefile.in (base_obj): Add back doprnt.o.
+
+        * deps.mk (doprnt.o): Add back prerequisites.
+        (callint.o): Depend on character.h.
+
+        * eval.c (internal_lisp_condition_case): Include the handler
+        representation in the error message.
+        (verror): Call doprnt instead of vsnprintf.  Fix an off-by-one bug
+        when breaking from the loop.
+
+        * xdisp.c (vmessage): Call doprnt instead of vsnprintf.
+
+        * callint.c (Fcall_interactively): When displaying error message
+        about invalid control letter, pass the character's codepoint, not
+        a pointer to its multibyte form.  Improve display of the character
+        in octal and display also its hex code.
+
+        * character.c (char_string): Use %x to display the (unsigned)
+        codepoint of an invalid character, to avoid displaying a bogus
+        negative value.
+
+        * font.c (check_otf_features): Pass SDATA of SYMBOL_NAME to
+        `error', not SYMBOL_NAME itself.
+
+        * coding.c (Fencode_sjis_char, Fencode_big5_char): Use %c for
+        character arguments to `error'.
+
+        * charset.c (check_iso_charset_parameter): Fix incorrect argument
+        to `error' in error message about FINAL_CHAR argument.  Make sure
+        FINAL_CHAR is a character, and use %c when it is passed as
+        argument to `error'.
+
+2011-04-23  Eli Zaretskii  <eliz@gnu.org>
+
         * s/ms-w32.h (localtime): Redirect to sys_localtime.
 
         * w32.c: Include <time.h>.
diff --git a/src/Makefile.in b/src/Makefile.in
index 154d6abba4e..e1195968f7f 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -354,7 +354,7 @@ base_obj = dispnew.o frame.o scroll.o xdisp.o menu.o $(XMENU_OBJ) window.o \
         syntax.o $(UNEXEC_OBJ) bytecode.o \
         process.o gnutls.o callproc.o \
         region-cache.o sound.o atimer.o \
-        intervals.o textprop.o composite.o xml.o \
+        doprnt.o intervals.o textprop.o composite.o xml.o \
         $(MSDOS_OBJ) $(MSDOS_X_OBJ) $(NS_OBJ) $(CYGWIN_OBJ) $(FONT_OBJ)
 obj = $(base_obj) $(NS_OBJC_OBJ)
 
diff --git a/src/callint.c b/src/callint.c
index e5ec3d7d931..cddd92c8a94 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -27,6 +27,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "keyboard.h"
 #include "window.h"
 #include "keymap.h"
+#include "character.h"
 
 Lisp_Object Qminus, Qplus;
 Lisp_Object Qcall_interactively;
@@ -786,8 +787,10 @@ invoke it.  If KEYS is omitted or nil, the return value of
              if anyone tries to define one here.  */
         case '+':
         default:
-          error ("Invalid control letter `%c' (%03o) in interactive calling string",
-                 *tem, (unsigned char) *tem);
+          error ("Invalid control letter `%c' (#o%03o, #x%04x) in interactive calling string",
+                 STRING_CHAR ((unsigned char *) tem),
+                 (unsigned) STRING_CHAR ((unsigned char *) tem),
+                 (unsigned) STRING_CHAR ((unsigned char *) tem));
         }
 
       if (varies[i] == 0)
diff --git a/src/character.c b/src/character.c
index 84eddeb2fc2..4087e8984d0 100644
--- a/src/character.c
+++ b/src/character.c
@@ -156,7 +156,7 @@ char_string (unsigned int c, unsigned char *p)
       bytes = BYTE8_STRING (c, p);
     }
   else
-    error ("Invalid character: %d", c);
+    error ("Invalid character: %x", c);
 
   return bytes;
 }
diff --git a/src/charset.c b/src/charset.c
index c4699dcb0a7..e7435c292e2 100644
--- a/src/charset.c
+++ b/src/charset.c
@@ -1436,7 +1436,7 @@ check_iso_charset_parameter (Lisp_Object dimension, Lisp_Object chars, Lisp_Obje
 {
   CHECK_NATNUM (dimension);
   CHECK_NATNUM (chars);
-  CHECK_NATNUM (final_char);
+  CHECK_CHARACTER (final_char);
 
   if (XINT (dimension) > 3)
     error ("Invalid DIMENSION %"pEd", it should be 1, 2, or 3",
@@ -1444,12 +1444,8 @@ check_iso_charset_parameter (Lisp_Object dimension, Lisp_Object chars, Lisp_Obje
   if (XINT (chars) != 94 && XINT (chars) != 96)
     error ("Invalid CHARS %"pEd", it should be 94 or 96", XINT (chars));
   if (XINT (final_char) < '0' || XINT (final_char) > '~')
-    {
-      unsigned char str[MAX_MULTIBYTE_LENGTH + 1];
-      int len = CHAR_STRING (XINT (chars), str);
-      str[len] = '\0';
-      error ("Invalid FINAL-CHAR %s, it should be `0'..`~'", str);
-    }
+    error ("Invalid FINAL-CHAR %c, it should be `0'..`~'",
+           (int)XINT (final_char));
 }
 
 
diff --git a/src/coding.c b/src/coding.c
index b49070e5e16..221ada51158 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -9071,7 +9071,7 @@ Return the corresponding code in SJIS.  */)
   charset_list = CODING_ATTR_CHARSET_LIST (attrs);
   charset = char_charset (c, charset_list, &code);
   if (code == CHARSET_INVALID_CODE (charset))
-    error ("Can't encode by shift_jis encoding: %d", c);
+    error ("Can't encode by shift_jis encoding: %c", c);
   JIS_TO_SJIS (code);
 
   return make_number (code);
@@ -9142,7 +9142,7 @@ Return the corresponding character code in Big5.  */)
   charset_list = CODING_ATTR_CHARSET_LIST (attrs);
   charset = char_charset (c, charset_list, &code);
   if (code == CHARSET_INVALID_CODE (charset))
-    error ("Can't encode by Big5 encoding: %d", c);
+    error ("Can't encode by Big5 encoding: %c", c);
 
   return make_number (code);
 }
diff --git a/src/deps.mk b/src/deps.mk
index 2df1577ef78..8d0e0e69589 100644
--- a/src/deps.mk
+++ b/src/deps.mk
@@ -44,7 +44,8 @@ buffer.o: buffer.c buffer.h region-cache.h commands.h window.h \
    $(INTERVALS_H) blockinput.h atimer.h systime.h character.h ../lib/unistd.h \
    indent.h keyboard.h coding.h keymap.h frame.h lisp.h globals.h $(config_h)
 callint.o: callint.c window.h commands.h buffer.h keymap.h globals.h msdos.h \
-   keyboard.h dispextern.h systime.h coding.h composite.h lisp.h $(config_h)
+   keyboard.h dispextern.h systime.h coding.h composite.h lisp.h \
+   character.h $(config_h)
 callproc.o: callproc.c epaths.h buffer.h commands.h lisp.h $(config_h) \
    process.h systty.h syssignal.h character.h coding.h ccl.h msdos.h \
    composite.h w32.h blockinput.h atimer.h systime.h frame.h termhooks.h \
@@ -82,6 +83,7 @@ dispnew.o: dispnew.c systime.h commands.h process.h frame.h coding.h \
 # doc.o's dependency on buildobj.h is in src/Makefile.in.
 doc.o: doc.c lisp.h $(config_h) buffer.h keyboard.h keymap.h \
    character.h systime.h coding.h composite.h ../lib/unistd.h globals.h
+doprnt.o: doprnt.c character.h lisp.h globals.h ../lib/unistd.h $(config_h)
 dosfns.o: buffer.h termchar.h termhooks.h frame.h blockinput.h window.h \
    msdos.h dosfns.h dispextern.h charset.h coding.h atimer.h systime.h \
    lisp.h $(config_h)
diff --git a/src/doprnt.c b/src/doprnt.c
index 36eb272caae..f182529b801 100644
--- a/src/doprnt.c
+++ b/src/doprnt.c
@@ -30,6 +30,11 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <unistd.h>
 
+#include <limits.h>
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
+
 #include "lisp.h"
 
 /* Since we use the macro CHAR_HEAD_P, we have to include this, but
@@ -51,8 +56,8 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
    String arguments are passed as C strings.
    Integers are passed as C integers.  */
 
-EMACS_INT
-doprnt (char *buffer, register int bufsize, const char *format,
+size_t
+doprnt (char *buffer, register size_t bufsize, const char *format,
         const char *format_end, va_list ap)
 {
   const char *fmt = format;     /* Pointer into format string */
@@ -62,15 +67,15 @@ doprnt (char *buffer, register int bufsize, const char *format,
   char tembuf[DBL_MAX_10_EXP + 100];
 
   /* Size of sprintf_buffer.  */
-  unsigned size_allocated = sizeof (tembuf);
+  size_t size_allocated = sizeof (tembuf);
 
   /* Buffer to use for sprintf.  Either tembuf or same as BIG_BUFFER.  */
   char *sprintf_buffer = tembuf;
 
   /* Buffer we have got with malloc.  */
-  char *big_buffer = 0;
+  char *big_buffer = NULL;
 
-  register int tem;
+  register size_t tem;
   char *string;
   char fixed_buffer[20];        /* Default buffer for small formatting. */
   char *fmtcpy;
@@ -92,8 +97,9 @@ doprnt (char *buffer, register int bufsize, const char *format,
     {
       if (*fmt == '%')  /* Check for a '%' character */
         {
-          unsigned size_bound = 0;
-          EMACS_INT width;  /* Columns occupied by STRING.  */
+          size_t size_bound = 0;
+          EMACS_INT width;  /* Columns occupied by STRING on display.  */
+          int long_flag = 0;
 
           fmt++;
           /* Copy this one %-spec into fmtcpy.  */
@@ -108,10 +114,11 @@ doprnt (char *buffer, register int bufsize, const char *format,
                      This might be a field width or a precision; e.g.
                      %1.1000f and %1000.1f both might need 1000+ bytes.
                      Parse the width or precision, checking for overflow.  */
-                  unsigned n = *fmt - '0';
+                  size_t n = *fmt - '0';
                   while ('0' <= fmt[1] && fmt[1] <= '9')
                     {
-                      if (n * 10 + fmt[1] - '0' < n)
+                      if (n >= SIZE_MAX / 10
+                          || n * 10 > SIZE_MAX - (fmt[1] - '0'))
                         error ("Format width or precision too large");
                       n = n * 10 + fmt[1] - '0';
                       *string++ = *++fmt;
@@ -122,6 +129,13 @@ doprnt (char *buffer, register int bufsize, const char *format,
                 }
               else if (*fmt == '-' || *fmt == ' ' || *fmt == '.' || *fmt == '+')
                 ;
+              else if (*fmt == 'l')
+                {
+                  long_flag = 1;
+                  if (!strchr ("dox", fmt[1]))
+                    /* %l as conversion specifier, not as modifier.  */
+                    break;
+                }
               else
                 break;
               fmt++;
@@ -130,7 +144,7 @@ doprnt (char *buffer, register int bufsize, const char *format,
 
           /* Make the size bound large enough to handle floating point formats
              with large numbers.  */
-          if (size_bound + DBL_MAX_10_EXP + 50 < size_bound)
+          if (size_bound > SIZE_MAX - DBL_MAX_10_EXP - 50)
             error ("Format width or precision too large");
           size_bound += DBL_MAX_10_EXP + 50;
 
@@ -151,23 +165,47 @@ doprnt (char *buffer, register int bufsize, const char *format,
               error ("Invalid format operation %%%c", fmt[-1]);
 
 /*          case 'b': */
+            case 'l':
             case 'd':
+              {
+                int i;
+                long l;
+
+                if (long_flag)
+                  {
+                    l = va_arg(ap, long);
+                    sprintf (sprintf_buffer, fmtcpy, l);
+                  }
+                else
+                  {
+                    i = va_arg(ap, int);
+                    sprintf (sprintf_buffer, fmtcpy, i);
+                  }
+                /* Now copy into final output, truncating as necessary.  */
+                string = sprintf_buffer;
+                goto doit;
+              }
+
             case 'o':
             case 'x':
-              if (sizeof (int) == sizeof (EMACS_INT))
-                ;
-              else if (sizeof (long) == sizeof (EMACS_INT))
-                /* Insert an `l' the right place.  */
-                string[1] = string[0],
-                string[0] = string[-1],
-                string[-1] = 'l',
-                string++;
-              else
-                abort ();
-              sprintf (sprintf_buffer, fmtcpy, va_arg(ap, char *));
-              /* Now copy into final output, truncating as nec.  */
-              string = sprintf_buffer;
-              goto doit;
+              {
+                unsigned u;
+                unsigned long ul;
+
+                if (long_flag)
+                  {
+                    ul = va_arg(ap, unsigned long);
+                    sprintf (sprintf_buffer, fmtcpy, ul);
+                  }
+                else
+                  {
+                    u = va_arg(ap, unsigned);
+                    sprintf (sprintf_buffer, fmtcpy, u);
+                  }
+                /* Now copy into final output, truncating as necessary.  */
+                string = sprintf_buffer;
+                goto doit;
+              }
 
             case 'f':
             case 'e':
@@ -175,7 +213,7 @@ doprnt (char *buffer, register int bufsize, const char *format,
               {
                 double d = va_arg(ap, double);
                 sprintf (sprintf_buffer, fmtcpy, d);
-                /* Now copy into final output, truncating as nec.  */
+                /* Now copy into final output, truncating as necessary.  */
                 string = sprintf_buffer;
                 goto doit;
               }
@@ -187,13 +225,18 @@ doprnt (char *buffer, register int bufsize, const char *format,
                 minlen = atoi (&fmtcpy[1]);
               string = va_arg (ap, char *);
               tem = strlen (string);
+              if (tem > MOST_POSITIVE_FIXNUM)
+                error ("String for %%s or %%S format is too long");
               width = strwidth (string, tem);
               goto doit1;
 
               /* Copy string into final output, truncating if no room.  */
             doit:
               /* Coming here means STRING contains ASCII only.  */
-              width = tem = strlen (string);
+              tem = strlen (string);
+              if (tem > MOST_POSITIVE_FIXNUM)
+                error ("Format width or precision too large");
+              width = tem;
             doit1:
               /* We have already calculated:
                  TEM -- length of STRING,
@@ -236,13 +279,8 @@ doprnt (char *buffer, register int bufsize, const char *format,
 
             case 'c':
               {
-                /* Sometimes for %c we pass a char, which would widen
-                   to int.  Sometimes we pass XFASTINT() or XINT()
-                   values, which would be EMACS_INT.  Let's hope that
-                   both are passed the same way, otherwise we'll need
-                   to rewrite callers.  */
-                EMACS_INT chr = va_arg(ap, EMACS_INT);
-                tem = CHAR_STRING ((int) chr, (unsigned char *) charbuf);
+                int chr = va_arg(ap, int);
+                tem = CHAR_STRING (chr, (unsigned char *) charbuf);
                 string = charbuf;
                 string[tem] = 0;
                 width = strwidth (string, tem);
@@ -274,6 +312,6 @@ doprnt (char *buffer, register int bufsize, const char *format,
   /* If we had to malloc something, free it.  */
   xfree (big_buffer);
 
-  *bufptr = 0;          /* Make sure our string end with a '\0' */
+  *bufptr = 0;          /* Make sure our string ends with a '\0' */
   return bufptr - buffer;
 }
diff --git a/src/eval.c b/src/eval.c
index b843ca5b2ec..c3676720940 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1416,7 +1416,8 @@ internal_lisp_condition_case (volatile Lisp_Object var, Lisp_Object bodyform,
              || (CONSP (tem)
                  && (SYMBOLP (XCAR (tem))
                      || CONSP (XCAR (tem))))))
-        error ("Invalid condition handler");
+        error ("Invalid condition handler: %s",
+               SDATA (Fprin1_to_string (tem, Qt)));
     }
 
   c.tag = Qnil;
@@ -1995,31 +1996,31 @@ verror (const char *m, va_list ap)
   size_t size = sizeof buf;
   size_t size_max =
     min (MOST_POSITIVE_FIXNUM, min (INT_MAX, SIZE_MAX - 1)) + 1;
+  size_t mlen = strlen (m);
   char *buffer = buf;
-  int used;
+  size_t used;
   Lisp_Object string;
 
   while (1)
     {
-      used = vsnprintf (buffer, size, m, ap);
+      used = doprnt (buffer, size, m, m + mlen, ap);
 
-      if (used < 0)
-        {
-          /* Non-C99 vsnprintf, such as w32, returns -1 when SIZE is too small.
-             Guess a larger USED to work around the incompatibility.  */
-          used = (size <= size_max / 2 ? 2 * size
-                  : size < size_max ? size_max - 1
-                  : size_max);
-        }
-      else if (used < size)
+      /* Note: the -1 below is because `doprnt' returns the number of bytes
+         excluding the terminating null byte, and it always terminates with a
+         null byte, even when producing a truncated message.  */
+      if (used < size - 1)
         break;
-      if (size_max <= used)
-        memory_full ();
-      size = used + 1;
+      if (size <= size_max / 2)
+        size *= 2;
+      else if (size < size_max - 1)
+        size = size_max - 1;
+      else
+        break;  /* and leave the message truncated */
 
-      if (buffer != buf)
-        xfree (buffer);
-      buffer = (char *) xmalloc (size);
+      if (buffer == buf)
+        buffer = (char *) xmalloc (size);
+      else
+        buffer = (char *) xrealloc (buffer, size);
     }
 
   string = make_string (buffer, used);
diff --git a/src/font.c b/src/font.c
index 7fe0815d80e..f4950d39189 100644
--- a/src/font.c
+++ b/src/font.c
@@ -1795,14 +1795,16 @@ check_otf_features (otf_features)
     {
       CHECK_SYMBOL (Fcar (val));
       if (SBYTES (SYMBOL_NAME (XCAR (val))) > 4)
-        error ("Invalid OTF GSUB feature: %s", SYMBOL_NAME (XCAR (val)));
+        error ("Invalid OTF GSUB feature: %s",
+               SDATA (SYMBOL_NAME (XCAR (val))));
     }
   otf_features = XCDR (otf_features);
   for (val = Fcar (otf_features); ! NILP (val);  val = Fcdr (val))
     {
       CHECK_SYMBOL (Fcar (val));
       if (SBYTES (SYMBOL_NAME (XCAR (val))) > 4)
-        error ("Invalid OTF GPOS feature: %s", SYMBOL_NAME (XCAR (val)));
+        error ("Invalid OTF GPOS feature: %s",
+               SDATA (SYMBOL_NAME (XCAR (val))));
     }
 }
 
diff --git a/src/lisp.h b/src/lisp.h
index 581835dd32b..07b2cb0b1ef 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -2760,6 +2760,9 @@ extern Lisp_Object internal_with_output_to_temp_buffer
 extern void float_to_string (char *, double);
 extern void syms_of_print (void);
 
+/* Defined in doprnt.c */
+extern size_t doprnt (char *, size_t, const char *, const char *, va_list);
+
 /* Defined in lread.c.  */
 extern Lisp_Object Qvariable_documentation, Qstandard_input;
 extern Lisp_Object Qbackquote, Qcomma, Qcomma_at, Qcomma_dot, Qfunction;
diff --git a/src/makefile.w32-in b/src/makefile.w32-in
index 62c40ca1f94..9d2c3d8f83f 100644
--- a/src/makefile.w32-in
+++ b/src/makefile.w32-in
@@ -469,6 +469,7 @@ $(BLD)/callint.$(O) : \
         $(EMACS_ROOT)/nt/inc/sys/time.h \
         $(LISP_H) \
         $(SRC)/buffer.h \
+        $(SRC)/character.h \
         $(SRC)/coding.h \
         $(SRC)/commands.h \
         $(SRC)/composite.h \
diff --git a/src/xdisp.c b/src/xdisp.c
index 19fef35fce8..91d1b6ea2e3 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -8373,22 +8373,10 @@ vmessage (const char *m, va_list ap)
         {
           if (m)
             {
-              char *buf = FRAME_MESSAGE_BUF (f);
-              size_t bufsize = FRAME_MESSAGE_BUF_SIZE (f);
-              int len;
+              size_t len;
 
-              memset (buf, 0, bufsize);
-              len = vsnprintf (buf, bufsize, m, ap);
-
-              /* Do any truncation at a character boundary.  */
-              if (! (0 <= len && len < bufsize))
-                {
-                  char *end = memchr (buf, 0, bufsize);
-                  for (len = end ? end - buf : bufsize;
-                       len && ! CHAR_HEAD_P (buf[len - 1]);
-                       len--)
-                    continue;
-                }
+              len = doprnt (FRAME_MESSAGE_BUF (f),
+                            FRAME_MESSAGE_BUF_SIZE (f), m, (char *)0, ap);
 
               message2 (FRAME_MESSAGE_BUF (f), len, 0);
             }