mysnprintf.c: Massive rewrite of PyOS_snprintf and PyOS_vsnprintf, to

use wrappers on all platforms, to make this as consistent as possible x-
platform (in particular, make sure there's at least one \0 byte in
the output buffer).  Also document more of the truth about what these do.

getargs.c, seterror():  Three computations of remaining buffer size were
backwards, thus telling PyOS_snprintf the buffer is larger than it
actually is.  This matters a lot now that PyOS_snprintf ensures there's a
trailing \0 byte (because it didn't get the truth about the buffer size,
it was storing \0 beyond the true end of the buffer).

sysmodule.c, mywrite():  Simplify, now that PyOS_vsnprintf guarantees to
produce a \0 byte.

Author: tim-one

Include/pyerrors.h

@@ -123,16 +123,11 @@ extern DL_IMPORT(PyObject *) PyErr_ProgramText(char *, int);
 # define vsnprintf _vsnprintf
 #endif
 
-#ifndef HAVE_SNPRINTF
 #include <stdarg.h>
 extern DL_IMPORT(int) PyOS_snprintf(char *str, size_t size, const char  *format, ...)
 			__attribute__((format(printf, 3, 4)));
 extern DL_IMPORT(int) PyOS_vsnprintf(char *str, size_t size, const char  *format, va_list va)
 			__attribute__((format(printf, 3, 0)));
-#else
-# define PyOS_vsnprintf	vsnprintf
-# define PyOS_snprintf	snprintf
-#endif
 
 #ifdef __cplusplus
 }

Python/getargs.c

@@ -231,7 +231,7 @@ seterror(int iarg, char *msg, int *levels, char *fname, char *message)
 			p += strlen(p);
 		}
 		if (iarg != 0) {
-			PyOS_snprintf(p, sizeof(buf) - (buf - p),
+			PyOS_snprintf(p, sizeof(buf) - (p - buf),
 				      "argument %d", iarg);
 			i = 0;
 			p += strlen(p);
@@ -243,10 +243,10 @@ seterror(int iarg, char *msg, int *levels, char *fname, char *message)
 			}
 		}
 		else {
-			PyOS_snprintf(p, sizeof(buf) - (buf - p), "argument");
+			PyOS_snprintf(p, sizeof(buf) - (p - buf), "argument");
 			p += strlen(p);
 		}
-		PyOS_snprintf(p, sizeof(buf) - (buf - p), " %.256s", msg);
+		PyOS_snprintf(p, sizeof(buf) - (p - buf), " %.256s", msg);
 		message = buf;
 	}
 	PyErr_SetString(PyExc_TypeError, message);

Python/mysnprintf.c

@@ -1,97 +1,93 @@
-
 #include "Python.h"
+#include <ctype.h>
 
-/* snprintf() emulation for platforms which don't have it (yet). 
-   
-   Return value
+/* snprintf() wrappers.  If the platform has vsnprintf, we use it, else we
+   emulate it in a half-hearted way.  Even if the platform has it, we wrap
+   it because platforms differ in what vsnprintf does in case the buffer
+   is too small:  C99 behavior is to return the number of characters that
+   would have been written had the buffer not been too small, and to set
+   the last byte of the buffer to \0.  At least MS _vsnprintf returns a
+   negative value instead, and fills the entire buffer with non-\0 data.
 
-       The number of characters printed (not including the trailing
-       `\0' used to end output to strings) or a negative number in
-       case of an error.
+   The wrappers ensure that str[size-1] is always \0 upon return.
 
-       PyOS_snprintf and PyOS_vsnprintf do not write more than size
-       bytes (including the trailing '\0').
+   PyOS_snprintf and PyOS_vsnprintf never write more than size bytes
+   (including the trailing '\0') into str.
 
-       If the output would have been truncated, they return the number
-       of characters (excluding the trailing '\0') which would have
-       been written to the final string if enough space had been
-       available. This is inline with the C99 standard.
+   If the platform doesn't have vsnprintf, and the buffer size needed to
+   avoid truncation exceeds size by more than 512, Python aborts with a
+   Py_FatalError.
 
-*/
+   Return value (rv):
 
-#include <ctype.h>
+	When 0 <= rv < size, the output conversion was unexceptional, and
+	rv characters were written to str (excluding a trailing \0 byte at
+	str[rv]).
 
-#ifndef HAVE_SNPRINTF
+	When rv >= size, output conversion was truncated, and a buffer of
+	size rv+1 would have been needed to avoid truncation.  str[size-1]
+	is \0 in this case.
 
-static
-int myvsnprintf(char *str, size_t size, const char  *format, va_list va)
-{
-    char *buffer = PyMem_Malloc(size + 512);
-    int len;
-    
-    if (buffer == NULL)
-	return -1;
-    len = vsprintf(buffer, format, va);
-    if (len < 0) {
-	PyMem_Free(buffer);
-	return len;
-    }
-    len++;
-    assert(len >= 0);
-    if ((size_t)len > size + 512)
-	Py_FatalError("Buffer overflow in PyOS_snprintf/PyOS_vsnprintf");
-    if ((size_t)len > size)
-	buffer[size-1] = '\0';
-    else
-	size = len;
-    memcpy(str, buffer, size);
-    PyMem_Free(buffer);
-    return len - 1;
-}
+	When rv < 0, "something bad happened".  str[size-1] is \0 in this
+	case too, but the rest of str is unreliable.  It could be that
+	an error in format codes was detected by libc, or on platforms
+	with a non-C99 vsnprintf simply that the buffer wasn't big enough
+	to avoid truncation, or on platforms without any vsnprintf that
+	PyMem_Malloc couldn't obtain space for a temp buffer.
+
+   CAUTION:  Unlike C99, str != NULL and size > 0 are required.
+*/
 
-int PyOS_snprintf(char *str, size_t size, const  char  *format, ...)
+int
+PyOS_snprintf(char *str, size_t size, const  char  *format, ...)
 {
-    int rc;
-    va_list va;
+	int rc;
+	va_list va;
 
-    va_start(va, format);
-    rc = myvsnprintf(str, size, format, va);
-    va_end(va);
-    return rc;
+	va_start(va, format);
+	rc = PyOS_vsnprintf(str, size, format, va);
+	va_end(va);
+	return rc;
 }
 
-int PyOS_vsnprintf(char *str, size_t size, const char  *format, va_list va)
+int
+PyOS_vsnprintf(char *str, size_t size, const char  *format, va_list va)
 {
-    return myvsnprintf(str, size, format, va);
-}
-
-#else
-
-/* Make sure that a C API is included in the lib */
-
-#ifdef PyOS_snprintf
-# undef PyOS_snprintf
+	int len;  /* # bytes written, excluding \0 */
+#ifndef HAVE_SNPRINTF
+	char *buffer;
 #endif
+	assert(str != NULL);
+	assert(size > 0);
+	assert(format != NULL);
 
-int PyOS_snprintf(char *str, size_t size, const  char  *format, ...)
-{
-    int rc;
-    va_list va;
-
-    va_start(va, format);
-    rc = vsnprintf(str, size, format, va);
-    va_end(va);
-    return rc;
-}
-
-#ifdef PyOS_vsnprintf
-# undef PyOS_vsnprintf
+#ifdef HAVE_SNPRINTF
+	len = vsnprintf(str, size, format, va);
+#else
+	/* Emulate it. */
+	buffer = PyMem_Malloc(size + 512);
+	if (buffer == NULL) {
+		len = -666;
+		goto Done;
+	}
+
+	len = vsprintf(buffer, format, va);
+	if (len < 0)
+		/* ignore the error */;
+
+	else if ((size_t)len >= size + 512)
+		Py_FatalError("Buffer overflow in PyOS_snprintf/PyOS_vsnprintf");
+
+	else {
+		const size_t to_copy = (size_t)len < size ?
+					(size_t)len : size - 1;
+		assert(to_copy < size);
+		memcpy(str, buffer, to_copy);
+		str[to_copy] = '\0';
+	}
+	PyMem_Free(buffer);
+Done:
 #endif
-
-int PyOS_vsnprintf(char *str, size_t size, const char  *format, va_list va)
-{
-    return vsnprintf(str, size, format, va);
+	str[size-1] = '\0';
+	return len;
 }
-
-#endif
-

Python/sysmodule.c

@@ -1025,18 +1025,11 @@ mywrite(char *name, FILE *fp, const char *format, va_list va)
 		char buffer[1001];
 		const int written = PyOS_vsnprintf(buffer, sizeof(buffer),
 						   format, va);
-		const int trouble = written < 0 || written >= sizeof(buffer);
-		if (trouble) {
-			/* Ensure there's a trailing null byte -- MS
-			   vsnprintf fills the buffer to the very end
-			   if it's not big enough. */
-			buffer[sizeof(buffer) - 1] = '\0';
-		}
 		if (PyFile_WriteString(buffer, file) != 0) {
 			PyErr_Clear();
 			fputs(buffer, fp);
 		}
-		if (trouble) {
+		if (written < 0 || written >= sizeof(buffer)) {
 			const char *truncated = "... truncated";
 			if (PyFile_WriteString(truncated, file) != 0) {
 				PyErr_Clear();