From e6c3da2065ac72cc4e1a2bef22d367cd75401892 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 23 Apr 2011 13:33:28 +0300 Subject: [PATCH] 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'. --- src/ChangeLog | 48 ++++++++++++++++++++ src/Makefile.in | 2 +- src/callint.c | 7 ++- src/character.c | 2 +- src/charset.c | 10 ++--- src/coding.c | 4 +- src/deps.mk | 4 +- src/doprnt.c | 106 ++++++++++++++++++++++++++++++-------------- src/eval.c | 37 ++++++++-------- src/font.c | 6 ++- src/lisp.h | 3 ++ src/makefile.w32-in | 1 + src/xdisp.c | 18 ++------ 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,3 +1,51 @@ +2011-04-23 Eli Zaretskii + + 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 * s/ms-w32.h (localtime): Redirect to sys_localtime. 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 . */ #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 . */ #include +#include +#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 . */ 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); } -- 2.39.2