Fix newly optimized strdup_vprintf.

As Jakub pointed out, our +1/-1 logic for null terminators wasn't quite right.
Also be sure to re-copy the va_list parameter if we need to re-use it, as
otherwise things break oddly.

Change-Id: Ibeaa95af602f565791e9378a6cfce434f13025eb
Reviewed-on: https://code.wireshark.org/review/1670
Reviewed-by: Evan Huus <eapache@gmail.com>

2 files changed, 11 insertions, 5 deletions

diff --git a/epan/wmem/wmem_strutl.c b/epan/wmem/wmem_strutl.c
index 38c3d0b34c..c35f2e68f0 100644
--- a/epan/wmem/wmem_strutl.c
+++ b/epan/wmem/wmem_strutl.c
@@ -39,7 +39,7 @@ wmem_strdup(wmem_allocator_t *allocator, const gchar *src)
 
     /* If the string is NULL, just return the string "<NULL>" so that the
      * callers don't have to bother checking it. */
-    if(!src) {
+    if (!src) {
         src = "<NULL>";
     }
 
@@ -100,16 +100,19 @@ wmem_strdup_vprintf(wmem_allocator_t *allocator, const gchar *fmt, va_list ap)
 
     dst = (gchar *)wmem_alloc(allocator, WMEM_STRDUP_VPRINTF_DEFAULT_BUFFER);
 
-    /* Returns: the number of characters which would be produced if the buffer was large enough (without NUL) */
+    /* Returns: the number of characters which would be produced if the buffer was large enough
+     * (not including the null, for which we add +1 ourselves). */
     needed_len = g_vsnprintf(dst, (gulong) WMEM_STRDUP_VPRINTF_DEFAULT_BUFFER, fmt, ap2) + 1;
+    va_end(ap2);
+
     if (needed_len > WMEM_STRDUP_VPRINTF_DEFAULT_BUFFER) {
         wmem_free(allocator, dst);
         dst = (gchar *)wmem_alloc(allocator, needed_len);
-        g_vsnprintf(dst, (gulong) needed_len, fmt, ap);
+        G_VA_COPY(ap2, ap);
+        g_vsnprintf(dst, (gulong) needed_len, fmt, ap2);
+        va_end(ap2);
     }
 
-    va_end(ap2);
-
     return dst;
 }
 
diff --git a/epan/wmem/wmem_test.c b/epan/wmem/wmem_test.c
index 0f999cb435..16cba1c529 100644
--- a/epan/wmem/wmem_test.c
+++ b/epan/wmem/wmem_test.c
@@ -33,6 +33,7 @@
 #include "wmem_allocator_simple.h"
 #include "wmem_allocator_strict.h"
 
+#define STRING_80               "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
 #define MAX_ALLOC_SIZE          (1024*64)
 #define MAX_SIMULTANEOUS_ALLOCS  1024
 #define CONTAINER_ITERS          10000
@@ -485,6 +486,8 @@ wmem_test_strutls(void)
 
     new_str = wmem_strdup_printf(allocator, "abc %s %% %d", "boo", 23);
     g_assert_cmpstr(new_str, ==, "abc boo % 23");
+    new_str = wmem_strdup_printf(allocator, "%s", STRING_80);
+    g_assert_cmpstr(new_str, ==, STRING_80);
     wmem_strict_check_canaries(allocator);
 
     new_str = wmem_strconcat(allocator, "ABC", NULL);