[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [Libevent-users] some trivial patches



On Tue, May 31, 2011 at 2:43 PM, Mansour Moufid <mansourmoufid@xxxxxxxxx> wrote:
> On Mon, May 30, 2011 at 11:02 AM, Nick Mathewson <nickm@xxxxxxxxxxxxx> wrote:
>> 0001 and 0002 look like non-starters.  They change a published
>> interface.  Code that followed the documented interface of
>> evhttp_*_set_max_*_size that  would have worked before will no longer
>> work with these changes applied.  We seriously try not to break
>> correct programs.
>
> Good point, scratch those.

Okay.

(Merged 0003 today.)

>> 0004 and the formatting part of 0005 look fine, but there's no actual
>> guarantee that a size_t can fit inside an unsigned long.  If we're
>> trying to fix that part of the code, we should fix it for real, and
>> format it properly.  (Do all the targets we care about support the %z
>> format, or do we need to get fancy?)
>
> %z would be supported anywhere C99 is. I think that might already be a
> requirement. Otherwise, using macros could work, something like so:
>
>  #if defined(__STDC__) && (__STDC_VERSION__ >= 199901L)
>  #define SIZE_FMT "%zu"
>  #define SIZE_FMT_CAST
>  #else
>  #define SIZE_FMT "%lu"
>  #define SIZE_FMT_CAST (unsigned long)
>  #endif

Not quite right, I think: The MSDN documentation implies that I
shouldn't expect %zu to work on Windows, and that sizeof(long) on
Windows is always 4 even when sizoef(size_t) is 8.

I've attached an annoyingly complex patch that just might work any
place; what do you think?

yrs,
-- 
Nick
From 3203f88c5f1fb13b2b3988ff0d1d802d70a143d9 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Wed, 8 Jun 2011 17:18:03 -0400
Subject: [PATCH] Use the correct printf args when formatting size_t

Based on a patch from Mansour Moufid
---
 http.c              |   24 +++++++++++++-----------
 test/regress_util.c |   18 ++++++++++++++++++
 util-internal.h     |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/http.c b/http.c
index a2bc570..e5cac8d 100644
--- a/http.c
+++ b/http.c
@@ -459,8 +459,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[22];
-		evutil_snprintf(size, sizeof(size), "%ld",
-		    (long)evbuffer_get_length(req->output_buffer));
+		evutil_snprintf(size, sizeof(size), EV_SIZE_FMT,
+		    EV_SIZE_ARG(evbuffer_get_length(req->output_buffer)));
 		evhttp_add_header(req->output_headers, "Content-Length", size);
 	}
 }
@@ -518,12 +518,13 @@ evhttp_maybe_add_date_header(struct evkeyvalq *headers)
  * unless it already has a content-length or transfer-encoding header. */
 static void
 evhttp_maybe_add_content_length_header(struct evkeyvalq *headers,
-    long content_length) /* XXX use size_t or int64, not long. */
+    size_t content_length)
 {
 	if (evhttp_find_header(headers, "Transfer-Encoding") == NULL &&
 	    evhttp_find_header(headers,	"Content-Length") == NULL) {
 		char len[22];
-		evutil_snprintf(len, sizeof(len), "%ld", content_length);
+		evutil_snprintf(len, sizeof(len), EV_SIZE_FMT,
+		    EV_SIZE_ARG(content_length));
 		evhttp_add_header(headers, "Content-Length", len);
 	}
 }
@@ -563,7 +564,7 @@ evhttp_make_header_response(struct evhttp_connection *evcon,
 			 */
 			evhttp_maybe_add_content_length_header(
 				req->output_headers,
-				(long)evbuffer_get_length(req->output_buffer));
+				evbuffer_get_length(req->output_buffer));
 		}
 	}
 
@@ -1076,9 +1077,10 @@ evhttp_read_cb(struct bufferevent *bufev, void *arg)
 
 			input = bufferevent_get_input(evcon->bufev);
 			total_len = evbuffer_get_length(input);
-			event_debug(("%s: read %d bytes in EVCON_IDLE state,"
-                                    " resetting connection",
-					__func__, (int)total_len));
+			event_debug(("%s: read "EV_SIZE_FMT
+				" bytes in EVCON_IDLE state,"
+				" resetting connection",
+				__func__, EV_SIZE_ARG(total_len)));
 #endif
 
 			evhttp_connection_reset(evcon);
@@ -1870,9 +1872,9 @@ evhttp_get_body_length(struct evhttp_request *req)
 		req->ntoread = ntoread;
 	}
 
-	event_debug(("%s: bytes to read: %ld (in buffer %ld)\n",
-		__func__, (long)req->ntoread,
-		evbuffer_get_length(bufferevent_get_input(req->evcon->bufev))));
+	event_debug(("%s: bytes to read: "EV_I64_FMT" (in buffer "EV_SIZE_FMT")\n",
+		__func__, EV_I64_ARG(req->ntoread),
+		EV_SIZE_ARG(evbuffer_get_length(bufferevent_get_input(req->evcon->bufev)))));
 
 	return (0);
 }
diff --git a/test/regress_util.c b/test/regress_util.c
index f31bb4a..f8ceb49 100644
--- a/test/regress_util.c
+++ b/test/regress_util.c
@@ -380,6 +380,11 @@ test_evutil_snprintf(void *ptr)
 {
 	char buf[16];
 	int r;
+	ev_uint64_t u64 = ((ev_uint64_t)1000000000)*200;
+	ev_uint64_t i64 = -1 * (ev_int64_t) u64;
+	size_t size = 8000;
+	ev_ssize_t ssize = -9000;
+
 	r = evutil_snprintf(buf, sizeof(buf), "%d %d", 50, 100);
 	tt_str_op(buf, ==, "50 100");
 	tt_int_op(r, ==, 6);
@@ -388,6 +393,19 @@ test_evutil_snprintf(void *ptr)
 	tt_str_op(buf, ==, "longish 1234567");
 	tt_int_op(r, ==, 18);
 
+	r = evutil_snprintf(buf, sizeof(buf), EV_U64_FMT, EV_U64_ARG(u64));
+	tt_str_op(buf, ==, "200000000000");
+	tt_int_op(r, ==, 12);
+
+	r = evutil_snprintf(buf, sizeof(buf), EV_I64_FMT, EV_I64_ARG(i64));
+	tt_str_op(buf, ==, "-200000000000");
+	tt_int_op(r, ==, 13);
+
+	r = evutil_snprintf(buf, sizeof(buf), EV_SIZE_FMT" "EV_SSIZE_FMT,
+	    EV_SIZE_ARG(size), EV_SSIZE_ARG(ssize));
+	tt_str_op(buf, ==, "8000 -9000");
+	tt_int_op(r, ==, 10);
+
       end:
 	;
 }
diff --git a/util-internal.h b/util-internal.h
index fe9ff35..2b4437f 100644
--- a/util-internal.h
+++ b/util-internal.h
@@ -270,6 +270,43 @@ int evutil_hex_char_to_int(char c);
 HANDLE evutil_load_windows_system_library(const TCHAR *library_name);
 #endif
 
+#ifndef EV_SIZE_FMT
+#if defined(_MSC_VER) || defined(__MINGW32__) || defined(__MINGW64__)
+#define EV_U64_FMT "%I64u"
+#define EV_I64_FMT "%I64d"
+#define EV_I64_ARG(x) ((__int64)(x))
+#define EV_U64_ARG(x) ((unsigned __int64)(x))
+#else
+#define EV_U64_FMT "%llu"
+#define EV_I64_FMT "%lld"
+#define EV_I64_ARG(x) ((long long)(x))
+#define EV_U64_ARG(x) ((unsigned long long)(x))
+#endif
+#endif
+
+#if defined(__STDC__) && defined(__STDC_VERSION__)
+#if (__STDC_VERSION__ >= 199901L)
+#define EV_SIZE_FMT "%zu"
+#define EV_SSIZE_FMT "%zd"
+#define EV_SIZE_ARG(x) (x)
+#define EV_SSIZE_ARG(x) (x)
+#endif
+#endif
+
+#ifndef EV_SIZE_FMT
+#if (_EVENT_SIZEOF_SIZE_T <= _EVENT_SIZEOF_LONG)
+#define EV_SIZE_FMT "%lu"
+#define EV_SSIZE_FMT "%ld"
+#define EV_SIZE_ARG(x) ((unsigned long)(x))
+#define EV_SSIZE_ARG(x) ((long)(x))
+#else
+#define EV_SIZE_FMT EV_U64_FMT
+#define EV_SSIZE_FMT EV_I64_FMT
+#define EV_SIZE_ARG(x) EV_U64_ARG(x)
+#define EV_SSIZE_ARG(x) EV_I64_ARG(x)
+#endif
+#endif
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.7.5.2