r19709@catbus: nickm | 2008-05-12 12:42:48 -0400

Possible fix for [1960723] snprintf and vsnprintf return values are wrong on win32


svn:r813

ChangeLog

@@ -98,7 +98,8 @@ Changes in current version:
  o Move	windows gettimeofday replacement into a new evutil_gettimeofday().
  o Make configure script work on IRIX.
  o provide a method for canceling ongoing http requests.
- 
+ o Make vsnprintf() returns consistent on win32.
+
 
 Changes in 1.4.0:
  o allow \r or \n individually to separate HTTP headers instead of the standard "\r\n"; from Charles Kerr.

buffer.c

@@ -954,12 +954,7 @@ evbuffer_add_vprintf(struct evbuffer *buf, const char *fmt, va_list ap)
 #endif
 		va_copy(aq, ap);
 
-#ifdef WIN32
-		sz = vsnprintf(buffer, space - 1, fmt, aq);
-		buffer[space - 1] = '\0';
-#else
-		sz = vsnprintf(buffer, space, fmt, aq);
-#endif
+		sz = evutil_vsnprintf(buffer, space, fmt, aq);
 
 		va_end(aq);
 

evdns.c

@@ -143,7 +143,6 @@ typedef unsigned int uint;
 #define u8  ev_uint8_t
 
 #ifdef WIN32
-#define snprintf _snprintf
 #define open _open
 #define read _read
 #define close _close
@@ -410,11 +409,11 @@ debug_ntoa(u32 address)
 {
 	static char buf[32];
 	u32 a = ntohl(address);
-	snprintf(buf, sizeof(buf), "%d.%d.%d.%d",
-                      (int)(u8)((a>>24)&0xff),
-                      (int)(u8)((a>>16)&0xff),
-                      (int)(u8)((a>>8 )&0xff),
-  		      (int)(u8)((a    )&0xff));
+	evutil_snprintf(buf, sizeof(buf), "%d.%d.%d.%d",
+					(int)(u8)((a>>24)&0xff),
+					(int)(u8)((a>>16)&0xff),
+					(int)(u8)((a>>8 )&0xff),
+					(int)(u8)((a    )&0xff));
 	return buf;
 }
 
@@ -441,12 +440,7 @@ _evdns_log(int warn, const char *fmt, ...)
   if (!evdns_log_fn)
     return;
   va_start(args,fmt);
-#ifdef WIN32
-  _vsnprintf(buf, sizeof(buf), fmt, args);
-#else
-  vsnprintf(buf, sizeof(buf), fmt, args);
-#endif
-  buf[sizeof(buf)-1] = '\0';
+  evutil_snprintf(buf, sizeof(buf), fmt, args);
   evdns_log_fn(warn, buf);
   va_end(args);
 }
@@ -738,7 +732,7 @@ reply_handle(struct evdns_request *const req, u16 flags, u32 ttl, struct reply *
 			/* we regard these errors as marking a bad nameserver */
 			if (req->reissue_count < req->base->global_max_reissues) {
 				char msg[64];
-				snprintf(msg, sizeof(msg), "Bad response %d (%s)",
+				evutil_snprintf(msg, sizeof(msg), "Bad response %d (%s)",
 					 error, evdns_err_to_string(error));
 				nameserver_failed(req->ns, msg);
 				if (!request_reissue(req)) return;
@@ -1600,7 +1594,7 @@ evdns_server_request_add_ptr_reply(struct evdns_server_request *req, struct in_a
 	assert(!(in && inaddr_name));
 	if (in) {
 		a = ntohl(in->s_addr);
-		snprintf(buf, sizeof(buf), "%d.%d.%d.%d.in-addr.arpa",
+		evutil_snprintf(buf, sizeof(buf), "%d.%d.%d.%d.in-addr.arpa",
 				(int)(u8)((a	)&0xff),
 				(int)(u8)((a>>8 )&0xff),
 				(int)(u8)((a>>16)&0xff),
@@ -2382,7 +2376,7 @@ evdns_base_resolve_reverse(struct evdns_base *base, struct in_addr *in, int flag
 	u32 a;
 	assert(in);
 	a = ntohl(in->s_addr);
-	snprintf(buf, sizeof(buf), "%d.%d.%d.%d.in-addr.arpa",
+	evutil_snprintf(buf, sizeof(buf), "%d.%d.%d.%d.in-addr.arpa",
 			(int)(u8)((a	)&0xff),
 			(int)(u8)((a>>8 )&0xff),
 			(int)(u8)((a>>16)&0xff),

evutil.c

@@ -49,6 +49,7 @@
 #include <stdlib.h>
 #endif
 #include <errno.h>
+#include <stdio.h>
 
 #ifndef _EVENT_HAVE_GETTIMEOFDAY
 #include <sys/timeb.h>
@@ -216,3 +217,31 @@ evutil_gettimeofday(struct timeval *tv, struct timezone *tz)
 	return 0;
 }
 #endif
+
+int
+evutil_snprintf(char *buf, size_t buflen, const char *format, ...)
+{
+	int r;
+	va_list ap;
+	va_start(ap, format);
+	r = vsnprintf(buf, buflen, format, ap);
+	va_end(ap);
+	return r;
+}
+
+int
+evutil_vsnprintf(char *buf, size_t buflen, const char *format, va_list ap)
+{
+#ifdef WIN32
+	int r = _vsnprintf(buf, buflen, format, ap);
+	buf[buflen-1] = '\0';
+	if (r >= 0)
+		return r;
+	else
+		return _vscprintf(format, ap);
+#else
+	int r = vsnprintf(buf, buflen, format, ap);
+	buf[buflen-1] = '\0';
+	return r;
+#endif
+}

http.c

@@ -96,7 +96,6 @@
 #include "mm-internal.h"
 
 #ifdef WIN32
-#define snprintf _snprintf
 #define strcasecmp _stricmp
 #define strncasecmp _strnicmp
 #endif
@@ -328,8 +327,8 @@ evhttp_make_header_request(struct evhttp_connection *evcon,
 	if ((req->type == EVHTTP_REQ_POST || req->type == EVHTTP_REQ_PUT) &&
 	    evhttp_find_header(req->output_headers, "Content-Length") == NULL){
 		char size[12];
-		snprintf(size, sizeof(size), "%ld",
-			 (long)EVBUFFER_LENGTH(req->output_buffer));
+		evutil_snprintf(size, sizeof(size), "%ld",
+						(long)EVBUFFER_LENGTH(req->output_buffer));
 		evhttp_add_header(req->output_headers, "Content-Length", size);
 	}
 }
@@ -385,7 +384,7 @@ evhttp_maybe_add_content_length_header(struct evkeyvalq *headers,
 	if (evhttp_find_header(headers, "Transfer-Encoding") == NULL &&
 	    evhttp_find_header(headers,	"Content-Length") == NULL) {
 		char len[12];
-		snprintf(len, sizeof(len), "%ld", content_length);
+		evutil_snprintf(len, sizeof(len), "%ld", content_length);
 		evhttp_add_header(headers, "Content-Length", len);
 	}
 }
@@ -2627,7 +2626,7 @@ make_addrinfo(const char *address, ev_uint16_t port)
         ai.ai_family = AF_INET;
         ai.ai_socktype = SOCK_STREAM;
         ai.ai_flags = AI_PASSIVE;  /* turn NULL host name into INADDR_ANY */
-        snprintf(strport, sizeof(strport), "%d", port);
+        evutil_snprintf(strport, sizeof(strport), "%d", port);
         if ((ai_result = getaddrinfo(address, strport, &ai, &aitop)) != 0) {
                 if ( ai_result == EAI_SYSTEM )
                         event_warn("getaddrinfo");

include/event2/util.h

@@ -53,6 +53,7 @@ extern "C" {
 #ifdef _EVENT_HAVE_STDDEF_H
 #include <stddef.h>
 #endif
+#include <stdarg.h>
 
 #ifdef _EVENT_HAVE_UINT64_T
 #define ev_uint64_t uint64_t
@@ -185,6 +186,19 @@ ev_int64_t evutil_strtoll(const char *s, char **endptr, int base);
 int evutil_gettimeofday(struct timeval *tv, struct timezone *tz);
 #endif
 
+#ifdef __GNUC__
+#define EVUTIL_CHECK_FMT(a,b) __attribute__((format(printf, a, b)))
+#else
+#define EVUTIL_CHECK_FMT(a,b)
+#endif
+
+int evutil_snprintf(char *buf, size_t buflen, const char *format, ...)
+#ifdef __GNUC__
+	__attribute__((format(printf, 3, 4)))
+#endif
+	;
+int evutil_vsnprintf(char *buf, size_t buflen, const char *format, va_list ap);
+
 #ifdef __cplusplus
 }
 #endif

log.c

@@ -58,6 +58,7 @@
 #include <string.h>
 #include <errno.h>
 #include "event2/event.h"
+#include "event2/util.h"
 
 #include "log.h"
 
@@ -65,37 +66,6 @@ static void _warn_helper(int severity, int log_errno, const char *fmt,
                          va_list ap);
 static void event_log(int severity, const char *msg);
 
-static int
-event_vsnprintf(char *str, size_t size, const char *format, va_list args)
-{
-	int r;
-	if (size == 0)
-		return -1;
-#ifdef WIN32
-	r = _vsnprintf(str, size, format, args);
-#else
-	r = vsnprintf(str, size, format, args);
-#endif
-	str[size-1] = '\0';
-	if (r < 0 || ((size_t)r) >= size) {
-		/* different platforms behave differently on overflow;
-		 * handle both kinds. */
-		return -1;
-	}
-	return r;
-}
-
-static int
-event_snprintf(char *str, size_t size, const char *format, ...)
-{
-    va_list ap;
-    int r;
-    va_start(ap, format);
-    r = event_vsnprintf(str, size, format, ap);
-    va_end(ap);
-    return r;
-}
-
 void
 event_err(int eval, const char *fmt, ...)
 {
@@ -165,14 +135,14 @@ _warn_helper(int severity, int log_errno, const char *fmt, va_list ap)
 	size_t len;
 
 	if (fmt != NULL)
-		event_vsnprintf(buf, sizeof(buf), fmt, ap);
+		evutil_vsnprintf(buf, sizeof(buf), fmt, ap);
 	else
 		buf[0] = '\0';
 
 	if (log_errno >= 0) {
 		len = strlen(buf);
 		if (len < sizeof(buf) - 3) {
-			event_snprintf(buf + len, sizeof(buf) - len, ": %s",
+			evutil_snprintf(buf + len, sizeof(buf) - len, ": %s",
 			    strerror(log_errno));
 		}
 	}

test/regress.c

@@ -1292,7 +1292,7 @@ test_evbuffer_iterative(void)
 		for (j = 1; j < strlen(abc); ++j) {
 			char format[32];
 
-			snprintf(format, sizeof(format), "%%%d.%ds", j, j);
+			evutil_snprintf(format, sizeof(format), "%%%d.%ds", j, j);
 			evbuffer_add_printf(buf, format, abc);
 			evbuffer_validate(buf);
 
@@ -1627,7 +1627,7 @@ test_priorities(int npriorities)
 	struct test_pri_event one, two;
 	struct timeval tv;
 
-	snprintf(buf, sizeof(buf), "Testing Priorities %d: ", npriorities);
+	evutil_snprintf(buf, sizeof(buf), "Testing Priorities %d: ", npriorities);
 	setup_test(buf);
 
 	event_base_priority_init(global_base, npriorities);
@@ -2066,6 +2066,38 @@ test_evutil_strtoll(void)
         cleanup_test();
 }
 
+static void
+test_evutil_snprintf(void)
+{
+	char buf[16];
+	int r;
+	setup_test("evutil_snprintf: ");
+	test_ok = 0;
+	r = evutil_snprintf(buf, sizeof(buf), "%d %d", 50, 100);
+	if (strcmp(buf, "50 100")) {
+		fprintf(stderr, "buf='%s'\n", buf);
+		goto err;
+	}
+	if (r != 6) {
+		fprintf(stderr, "r=%d\n", r);
+		goto err;
+	}
+
+	r = evutil_snprintf(buf, sizeof(buf), "longish %d", 1234567890);
+	if (strcmp(buf, "longish 1234567")) {
+		fprintf(stderr, "buf='%s'\n", buf);
+		goto err;
+	}
+	if (r != 18) {
+		fprintf(stderr, "r=%d\n", r);
+		goto err;
+	}
+
+	test_ok = 1;
+ err:
+	cleanup_test();
+}
+
 static void
 test_methods(void)
 {
@@ -2130,7 +2162,8 @@ main (int argc, char **argv)
 	/* Initalize the event library */
 	global_base = event_init();
 
-        test_evutil_strtoll();
+	test_evutil_strtoll();
+	test_evutil_snprintf();
 
 	test_periodictimeout();
 

test/regress_http.c

@@ -138,7 +138,7 @@ http_connect(const char *address, u_short port)
 	memset(&ai, 0, sizeof (ai));
 	ai.ai_family = AF_INET;
 	ai.ai_socktype = SOCK_STREAM;
-	snprintf(strport, sizeof (strport), "%d", port);
+	evutil_snprintf(strport, sizeof (strport), "%d", port);
 	if (getaddrinfo(address, strport, &ai, &aitop) != 0) {
 		event_warn("getaddrinfo");
 		return (-1);