From 8666506ecd6b1a90c7c66fb4b6051e90fba20c30 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 1 Sep 2011 07:44:49 -0700
Subject: [PATCH] * src/doprnt.c (esnprintf): Remove. All uses removed.
Suggested by Chong Yidong in <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9412#23>.
---
src/ChangeLog | 11 +++---
src/dbusbind.c | 42 ++++++++++++++--------
src/dispnew.c | 20 +++++------
src/doprnt.c | 21 -----------
src/font.c | 94 ++++++++++++++++++++++++++++++++++++--------------
src/lisp.h | 2 --
src/xterm.c | 8 ++---
7 files changed, 115 insertions(+), 83 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 0ba2df42186..94ba3a040ca 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,9 +1,9 @@
-2011-08-31 Paul Eggert <eggert@cs.ucla.edu>
+2011-09-01 Paul Eggert <eggert@cs.ucla.edu>
sprintf-related integer and memory overflow issues (Bug#9412).
* doprnt.c (doprnt): Support printing ptrdiff_t and intmax_t values.
- (esprintf, esnprintf, exprintf, evxprintf): New functions.
+ (esprintf, exprintf, evxprintf): New functions.
* keyboard.c (command_loop_level): Now EMACS_INT, not int.
(cmd_error): kbd macro iterations count is now EMACS_INT, not int.
(modify_event_symbol): Do not assume that the length of
@@ -17,7 +17,7 @@
* minibuf.c (minibuf_level): Now EMACS_INT, not int.
(get_minibuffer): Arg is now EMACS_INT, not int.
* lisp.h (get_minibuffer, push_key_description): Reflect API changes.
- (esprintf, esnprintf, exprintf, evxprintf): New decls.
+ (esprintf, exprintf, evxprintf): New decls.
* window.h (command_loop_level, minibuf_level): Reflect API changes.
* dbusbind.c (signature_cat): New function.
@@ -43,7 +43,7 @@
* font.c: Include <float.h>, for DBL_MAX_10_EXP.
(font_unparse_xlfd): Don't blindly alloca long strings.
Don't assume XINT result fits in int, or that XFLOAT_DATA * 10
- fits in int, when using sprintf. Use single esnprintf to count
+ fits in int, when using sprintf. Use single snprintf to count
length of string rather than counting it via multiple sprintfs;
that's simpler and more reliable.
(font_unparse_fcname): Use it to avoid sprintf buffer overrun.
@@ -102,9 +102,6 @@
to avoid potential buffer overrun.
* xterm.c (x_io_error_quitter): Don't overrun sprintf buffer.
- (x_term_init): Use sprintf, not snprintf, so that we need not
- worry about ancient hosts that lack snprintf. The buffer cannot
- possibly be overrun, so this is safe.
* xterm.h (x_check_errors): Add ATTRIBUTE_FORMAT_PRINTF.
diff --git a/src/dbusbind.c b/src/dbusbind.c
index fd9a43aaf86..8dac2a6249f 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -284,6 +284,7 @@ xd_signature (char *signature, unsigned int dtype, unsigned int parent_type, Lis
unsigned int subtype;
Lisp_Object elt;
char const *subsig;
+ int subsiglen;
char x[DBUS_MAXIMUM_SIGNATURE_LENGTH];
elt = object;
@@ -365,9 +366,9 @@ xd_signature (char *signature, unsigned int dtype, unsigned int parent_type, Lis
elt = CDR_SAFE (XD_NEXT_VALUE (elt));
}
- if (esnprintf (signature, DBUS_MAXIMUM_SIGNATURE_LENGTH,
- "%c%s", dtype, subsig)
- == DBUS_MAXIMUM_SIGNATURE_LENGTH - 1)
+ subsiglen = snprintf (signature, DBUS_MAXIMUM_SIGNATURE_LENGTH,
+ "%c%s", dtype, subsig);
+ if (! (0 <= subsiglen && subsiglen < DBUS_MAXIMUM_SIGNATURE_LENGTH))
string_overflow ();
break;
@@ -2088,32 +2089,45 @@ usage: (dbus-register-signal BUS SERVICE PATH INTERFACE SIGNAL HANDLER &rest ARG
connection = xd_initialize (bus, TRUE);
/* Create a rule to receive related signals. */
- rulelen = esnprintf (rule, sizeof rule,
- "type='signal',interface='%s',member='%s'",
- SDATA (interface),
- SDATA (signal));
+ rulelen = snprintf (rule, sizeof rule,
+ "type='signal',interface='%s',member='%s'",
+ SDATA (interface),
+ SDATA (signal));
+ if (! (0 <= rulelen && rulelen < sizeof rule))
+ string_overflow ();
/* Add unique name and path to the rule if they are non-nil. */
if (!NILP (uname))
- rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen,
+ {
+ int len = snprintf (rule + rulelen, sizeof rule - rulelen,
",sender='%s'", SDATA (uname));
+ if (! (0 <= len && len < sizeof rule - rulelen))
+ string_overflow ();
+ rulelen += len;
+ }
if (!NILP (path))
- rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen,
+ {
+ int len = snprintf (rule + rulelen, sizeof rule - rulelen,
",path='%s'", SDATA (path));
+ if (! (0 <= len && len < sizeof rule - rulelen))
+ string_overflow ();
+ rulelen += len;
+ }
/* Add arguments to the rule if they are non-nil. */
for (i = 6; i < nargs; ++i)
if (!NILP (args[i]))
{
+ int len;
CHECK_STRING (args[i]);
- rulelen += esnprintf (rule + rulelen, sizeof rule - rulelen,
- ",arg%"pD"d='%s'", i - 6, SDATA (args[i]));
+ len = snprintf (rule + rulelen, sizeof rule - rulelen,
+ ",arg%"pD"d='%s'", i - 6, SDATA (args[i]));
+ if (! (0 <= len && len < sizeof rule - rulelen))
+ string_overflow ();
+ rulelen += len;
}
- if (rulelen == sizeof rule - 1)
- string_overflow ();
-
/* Add the rule to the bus. */
dbus_error_init (&derror);
dbus_bus_add_match (connection, rule, &derror);
diff --git a/src/dispnew.c b/src/dispnew.c
index 0cc888b4b7a..5c28d014819 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -272,16 +272,16 @@ add_window_display_history (struct window *w, const char *msg, int paused_p)
buf = redisplay_history[history_idx].trace;
++history_idx;
- esnprintf (buf, sizeof redisplay_history[0].trace,
- "%"pMu": window %p (`%s')%s\n%s",
- history_tick++,
- w,
- ((BUFFERP (w->buffer)
- && STRINGP (BVAR (XBUFFER (w->buffer), name)))
- ? SSDATA (BVAR (XBUFFER (w->buffer), name))
- : "???"),
- paused_p ? " ***paused***" : "",
- msg);
+ snprintf (buf, sizeof redisplay_history[0].trace,
+ "%"pMu": window %p (`%s')%s\n%s",
+ history_tick++,
+ w,
+ ((BUFFERP (w->buffer)
+ && STRINGP (BVAR (XBUFFER (w->buffer), name)))
+ ? SSDATA (BVAR (XBUFFER (w->buffer), name))
+ : "???"),
+ paused_p ? " ***paused***" : "",
+ msg);
}
diff --git a/src/doprnt.c b/src/doprnt.c
index dae1dab04d7..638fa4d6312 100644
--- a/src/doprnt.c
+++ b/src/doprnt.c
@@ -486,27 +486,6 @@ esprintf (char *buf, char const *format, ...)
return nbytes;
}
-/* Format to a buffer BUF of positive size BUFSIZE. This is like
- snprintf, except it is not limited to returning an 'int' so it
- doesn't have a silly 2 GiB limit on typical 64-bit hosts. However,
- it is limited to the Emacs-style formats that doprnt supports, and
- BUFSIZE must be positive.
-
- Return the number of bytes put into BUF, excluding the terminating
- '\0'. Unlike snprintf, always return a nonnegative value less than
- BUFSIZE; if the output is truncated, return BUFSIZE - 1, which is
- the length of the truncated output. */
-ptrdiff_t
-esnprintf (char *buf, ptrdiff_t bufsize, char const *format, ...)
-{
- ptrdiff_t nbytes;
- va_list ap;
- va_start (ap, format);
- nbytes = doprnt (buf, bufsize, format, 0, ap);
- va_end (ap);
- return nbytes;
-}
-
/* Format to buffer *BUF of positive size *BUFSIZE, reallocating *BUF
and updating *BUFSIZE if the buffer is too small, and otherwise
behaving line esprintf. When reallocating, free *BUF unless it is
diff --git a/src/font.c b/src/font.c
index a5b873aea51..34cacb37ce4 100644
--- a/src/font.c
+++ b/src/font.c
@@ -1285,14 +1285,14 @@ font_unparse_xlfd (Lisp_Object font, int pixel_size, char *name, int nbytes)
}
else
f[XLFD_AVGWIDTH_INDEX] = "*";
- len = esnprintf (name, nbytes, "-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s",
- f[XLFD_FOUNDRY_INDEX], f[XLFD_FAMILY_INDEX],
- f[XLFD_WEIGHT_INDEX], f[XLFD_SLANT_INDEX],
- f[XLFD_SWIDTH_INDEX], f[XLFD_ADSTYLE_INDEX],
- f[XLFD_PIXEL_INDEX], f[XLFD_RESX_INDEX],
- f[XLFD_SPACING_INDEX], f[XLFD_AVGWIDTH_INDEX],
- f[XLFD_REGISTRY_INDEX]);
- return len == nbytes - 1 ? -1 : len;
+ len = snprintf (name, nbytes, "-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s-%s",
+ f[XLFD_FOUNDRY_INDEX], f[XLFD_FAMILY_INDEX],
+ f[XLFD_WEIGHT_INDEX], f[XLFD_SLANT_INDEX],
+ f[XLFD_SWIDTH_INDEX], f[XLFD_ADSTYLE_INDEX],
+ f[XLFD_PIXEL_INDEX], f[XLFD_RESX_INDEX],
+ f[XLFD_SPACING_INDEX], f[XLFD_AVGWIDTH_INDEX],
+ f[XLFD_REGISTRY_INDEX]);
+ return len < nbytes ? len : -1;
}
/* Parse NAME (null terminated) and store information in FONT
@@ -1593,31 +1593,75 @@ font_unparse_fcname (Lisp_Object font, int pixel_size, char *name, int nbytes)
p = name;
lim = name + nbytes;
if (! NILP (family))
- p += esnprintf (p, lim - p, "%s", SSDATA (family));
+ {
+ int len = snprintf (p, lim - p, "%s", SSDATA (family));
+ if (! (0 <= len && len < lim - p))
+ return -1;
+ p += len;
+ }
if (point_size > 0)
- p += esnprintf (p, lim - p, "-%d" + (p == name), point_size);
+ {
+ int len = snprintf (p, lim - p, "-%d" + (p == name), point_size);
+ if (! (0 <= len && len < lim - p))
+ return -1;
+ p += len;
+ }
else if (pixel_size > 0)
- p += esnprintf (p, lim - p, ":pixelsize=%d", pixel_size);
+ {
+ int len = snprintf (p, lim - p, ":pixelsize=%d", pixel_size);
+ if (! (0 <= len && len < lim - p))
+ return -1;
+ p += len;
+ }
if (! NILP (AREF (font, FONT_FOUNDRY_INDEX)))
- p += esnprintf (p, lim - p, ":foundry=%s",
- SSDATA (SYMBOL_NAME (AREF (font,
- FONT_FOUNDRY_INDEX))));
+ {
+ int len = snprintf (p, lim - p, ":foundry=%s",
+ SSDATA (SYMBOL_NAME (AREF (font,
+ FONT_FOUNDRY_INDEX))));
+ if (! (0 <= len && len < lim - p))
+ return -1;
+ p += len;
+ }
for (i = 0; i < 3; i++)
if (! NILP (styles[i]))
- p += esnprintf (p, lim - p, ":%s=%s", style_names[i],
- SSDATA (SYMBOL_NAME (styles[i])));
+ {
+ int len = snprintf (p, lim - p, ":%s=%s", style_names[i],
+ SSDATA (SYMBOL_NAME (styles[i])));
+ if (! (0 <= len && len < lim - p))
+ return -1;
+ p += len;
+ }
+
if (INTEGERP (AREF (font, FONT_DPI_INDEX)))
- p += esnprintf (p, lim - p, ":dpi=%"pI"d",
- XINT (AREF (font, FONT_DPI_INDEX)));
+ {
+ int len = snprintf (p, lim - p, ":dpi=%"pI"d",
+ XINT (AREF (font, FONT_DPI_INDEX)));
+ if (! (0 <= len && len < lim - p))
+ return -1;
+ p += len;
+ }
+
if (INTEGERP (AREF (font, FONT_SPACING_INDEX)))
- p += esnprintf (p, lim - p, ":spacing=%"pI"d",
- XINT (AREF (font, FONT_SPACING_INDEX)));
+ {
+ int len = snprintf (p, lim - p, ":spacing=%"pI"d",
+ XINT (AREF (font, FONT_SPACING_INDEX)));
+ if (! (0 <= len && len < lim - p))
+ return -1;
+ p += len;
+ }
+
if (INTEGERP (AREF (font, FONT_AVGWIDTH_INDEX)))
- p += esnprintf (p, lim - p,
- (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0
- ? ":scalable=true"
- : ":scalable=false"));
- return lim - p == 1 ? -1 : p - name;
+ {
+ int len = snprintf (p, lim - p,
+ (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0
+ ? ":scalable=true"
+ : ":scalable=false"));
+ if (! (0 <= len && len < lim - p))
+ return -1;
+ p += len;
+ }
+
+ return (p - name);
}
/* Parse NAME (null terminated) and store information in FONT
diff --git a/src/lisp.h b/src/lisp.h
index 2f6ec38f228..5c84bb8e06e 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -2897,8 +2897,6 @@ extern ptrdiff_t doprnt (char *, ptrdiff_t, const char *, const char *,
va_list);
extern ptrdiff_t esprintf (char *, char const *, ...)
ATTRIBUTE_FORMAT_PRINTF (2, 3);
-extern ptrdiff_t esnprintf (char *, ptrdiff_t, char const *, ...)
- ATTRIBUTE_FORMAT_PRINTF (3, 4);
extern ptrdiff_t exprintf (char **, ptrdiff_t *, char const *, ptrdiff_t,
char const *, ...)
ATTRIBUTE_FORMAT_PRINTF (5, 6);
diff --git a/src/xterm.c b/src/xterm.c
index 72e9f2b2236..86393cf411f 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -7900,8 +7900,8 @@ x_io_error_quitter (Display *display)
{
char buf[256];
- esnprintf (buf, sizeof buf, "Connection lost to X server `%s'",
- DisplayString (display));
+ snprintf (buf, sizeof buf, "Connection lost to X server `%s'",
+ DisplayString (display));
x_connection_closed (display, buf);
return 0;
}
@@ -10278,8 +10278,8 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name)
atom_names[i] = (char *) atom_refs[i].name;
/* Build _XSETTINGS_SN atom name */
- sprintf (xsettings_atom_name,
- "_XSETTINGS_S%d", XScreenNumberOfScreen (dpyinfo->screen));
+ snprintf (xsettings_atom_name, sizeof (xsettings_atom_name),
+ "_XSETTINGS_S%d", XScreenNumberOfScreen (dpyinfo->screen));
atom_names[i] = xsettings_atom_name;
XInternAtoms (dpyinfo->display, atom_names, total_atom_count,
--
2.39.5