emacs-diffs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Emacs-diffs] /srv/bzr/emacs/trunk r103979: Fix doprnt so it could be us


From: Eli Zaretskii
Subject: [Emacs-diffs] /srv/bzr/emacs/trunk r103979: Fix doprnt so it could be used safely in `verror'. (Bug#8435)
Date: Sat, 23 Apr 2011 13:33:28 +0300
User-agent: Bazaar (2.3.1)

------------------------------------------------------------
revno: 103979
fixes bug(s): http://debbugs.gnu.org/8435
committer: Eli Zaretskii <address@hidden>
branch nick: trunk
timestamp: Sat 2011-04-23 13:33:28 +0300
message:
  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'.
modified:
  src/ChangeLog
  src/Makefile.in
  src/callint.c
  src/character.c
  src/charset.c
  src/coding.c
  src/deps.mk
  src/doprnt.c
  src/eval.c
  src/font.c
  src/lisp.h
  src/makefile.w32-in
  src/xdisp.c
=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2011-04-23 03:07:16 +0000
+++ b/src/ChangeLog     2011-04-23 10:33:28 +0000
@@ -1,5 +1,53 @@
 2011-04-23  Eli Zaretskii  <address@hidden>
 
+       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  <address@hidden>
+
        * s/ms-w32.h (localtime): Redirect to sys_localtime.
 
        * w32.c: Include <time.h>.

=== modified file 'src/Makefile.in'
--- a/src/Makefile.in   2011-04-07 03:49:25 +0000
+++ b/src/Makefile.in   2011-04-23 10:33:28 +0000
@@ -354,7 +354,7 @@
        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)
 

=== modified file 'src/callint.c'
--- a/src/callint.c     2011-04-14 05:04:02 +0000
+++ b/src/callint.c     2011-04-23 10:33:28 +0000
@@ -27,6 +27,7 @@
 #include "keyboard.h"
 #include "window.h"
 #include "keymap.h"
+#include "character.h"
 
 Lisp_Object Qminus, Qplus;
 Lisp_Object Qcall_interactively;
@@ -786,8 +787,10 @@
             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)

=== modified file 'src/character.c'
--- a/src/character.c   2011-04-14 05:04:02 +0000
+++ b/src/character.c   2011-04-23 10:33:28 +0000
@@ -156,7 +156,7 @@
       bytes = BYTE8_STRING (c, p);
     }
   else
-    error ("Invalid character: %d", c);
+    error ("Invalid character: %x", c);
 
   return bytes;
 }

=== modified file 'src/charset.c'
--- a/src/charset.c     2011-04-14 20:16:48 +0000
+++ b/src/charset.c     2011-04-23 10:33:28 +0000
@@ -1436,7 +1436,7 @@
 {
   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 @@
   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));
 }
 
 

=== modified file 'src/coding.c'
--- a/src/coding.c      2011-04-14 05:04:02 +0000
+++ b/src/coding.c      2011-04-23 10:33:28 +0000
@@ -9071,7 +9071,7 @@
   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 @@
   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);
 }

=== modified file 'src/deps.mk'
--- a/src/deps.mk       2011-04-07 03:49:25 +0000
+++ b/src/deps.mk       2011-04-23 10:33:28 +0000
@@ -44,7 +44,8 @@
    $(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 @@
 # 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)

=== modified file 'src/doprnt.c'
--- a/src/doprnt.c      2011-04-10 16:44:27 +0000
+++ b/src/doprnt.c      2011-04-23 10:33:28 +0000
@@ -30,6 +30,11 @@
 
 #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 @@
    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 @@
   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 @@
     {
       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 @@
                     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 @@
                }
              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 @@
 
          /* 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 @@
              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 @@
              {
                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 @@
                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 @@
 
            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 @@
   /* 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;
 }

=== modified file 'src/eval.c'
--- a/src/eval.c        2011-04-16 21:50:01 +0000
+++ b/src/eval.c        2011-04-23 10:33:28 +0000
@@ -1416,7 +1416,8 @@
             || (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 @@
   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);

=== modified file 'src/font.c'
--- a/src/font.c        2011-04-15 01:26:32 +0000
+++ b/src/font.c        2011-04-23 10:33:28 +0000
@@ -1795,14 +1795,16 @@
     {
       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))));
     }
 }
 

=== modified file 'src/lisp.h'
--- a/src/lisp.h        2011-04-15 08:22:34 +0000
+++ b/src/lisp.h        2011-04-23 10:33:28 +0000
@@ -2760,6 +2760,9 @@
 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;

=== modified file 'src/makefile.w32-in'
--- a/src/makefile.w32-in       2011-03-19 03:22:14 +0000
+++ b/src/makefile.w32-in       2011-04-23 10:33:28 +0000
@@ -469,6 +469,7 @@
        $(EMACS_ROOT)/nt/inc/sys/time.h \
        $(LISP_H) \
        $(SRC)/buffer.h \
+       $(SRC)/character.h \
        $(SRC)/coding.h \
        $(SRC)/commands.h \
        $(SRC)/composite.h \

=== modified file 'src/xdisp.c'
--- a/src/xdisp.c       2011-04-23 03:07:16 +0000
+++ b/src/xdisp.c       2011-04-23 10:33:28 +0000
@@ -8373,22 +8373,10 @@
        {
          if (m)
            {
-             char *buf = FRAME_MESSAGE_BUF (f);
-             size_t bufsize = FRAME_MESSAGE_BUF_SIZE (f);
-             int 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;
-               }
+             size_t len;
+
+             len = doprnt (FRAME_MESSAGE_BUF (f),
+                           FRAME_MESSAGE_BUF_SIZE (f), m, (char *)0, ap);
 
              message2 (FRAME_MESSAGE_BUF (f), len, 0);
            }


reply via email to

[Prev in Thread] Current Thread [Next in Thread]