Browse Source

Fix cross platform details in json_printf

1. support native size_t and (u)int64_t formatters
2. fix %lld on 32-bit unices
3. fix x86 32-bit (e.g. linux and 32-bit osx) var_arg portability

The trick used to move to the next argument on windows was wrong:
a) it was marked as a windows-only hack but instead it would affect any
x86-32 ABI because va_arg there is a simple pointer and thus passed by
value. And so it would likely be on any platform where arguments are
solely passed on the stack. Anyway, it's compiler and ABI dependent.
b) it assumed that any type was wide as `void*` which obviously is not
true, e.g.: in `json_printf(out, "%lld %d", a, b)` if we consume a void*
when processing `a` we won't land on `b`.

The solution is to actually parse the format.
We're more likely to get all the printf types right than to get all
the embedded platform vararg right. Or, to put it differently: we care
less if we get obscure formats wrong, but we care if on a new port the
basic %c, %d, %zu, %lu, %llu formats suddenly don't work.

This could turn into a complete fprintf shim. I left the part that delegates
the actual modifier parsing to system snprintf.

Added 32bit unix test

PUBLISHED_FROM=e53f30b385c63050e74f641983c2f9abcc06c336
Marko Mikulicic 9 years ago
parent
commit
369f33b662
3 changed files with 158 additions and 15 deletions
  1. 2 1
      Makefile
  2. 94 14
      frozen.c
  3. 62 0
      unit_test.c

+ 2 - 1
Makefile

@@ -3,11 +3,12 @@ CFLAGS = -W -Wall -pedantic -O3 $(CFLAGS_EXTRA)
 
 all:
 	cc unit_test.c -o unit_test $(CFLAGS) && ./unit_test
+	cc -m32 unit_test.c -o unit_test $(CFLAGS) && ./unit_test
 	g++ unit_test.c -o unit_test $(CFLAGS) && ./unit_test
 	gcov -a unit_test.c
 
 w:
-	wine cl unit_test.c && wine unit_test.exe
+	wine cl /DEBUG unit_test.c && wine unit_test.exe
 
 clean:
 	rm -rf *.gc* *.dSYM unit_test unit_test.exe

+ 94 - 14
frozen.c

@@ -27,18 +27,35 @@
 #include <string.h>
 
 #ifdef _WIN32
-#define snprintf _snprintf
-#define vsnprintf _vsnprintf
+#define snprintf cs_win_snprintf
+#define vsnprintf cs_win_vsnprintf
+int cs_win_snprintf(char *str, size_t size, const char *format, ...);
+int cs_win_vsnprintf(char *str, size_t size, const char *format, va_list ap);
+#if _MSC_VER >= 1700
+#include <stdint.h>
+#else
 typedef _int64 int64_t;
-#define INT64_FMT "%I64"
-#define UINT64_FMT "%I64"
-#define CONSUME_ARG(ap) (void) va_arg(ap, void *);
-#define va_copy(x, y) x = y
+typedef unsigned _int64 uint64_t;
+#endif
+#define PRId64 "I64d"
+#define PRIu64 "I64u"
+#if _MSC_VER >= 1310
+#define SIZE_T_FMT "Iu"
 #else
-#define INT64_FMT "%lld"
-#define UINT64_FMT "%llu"
-#define CONSUME_ARG(ap)
+#define SIZE_T_FMT "u"
+#endif
+#define va_copy(x, y) x = y
+#else /* _WIN32 */
+/* <inttypes.h> wants this for C++ */
+#ifndef __STDC_FORMAT_MACROS
+#define __STDC_FORMAT_MACROS
 #endif
+#include <inttypes.h>
+#define SIZE_T_FMT "zu"
+#endif /* _WIN32 */
+
+#define INT64_FMT PRId64
+#define UINT64_FMT PRIu64
 
 #ifndef FROZEN_REALLOC
 #define FROZEN_REALLOC realloc
@@ -500,14 +517,20 @@ int json_vprintf(struct json_out *out, const char *fmt, va_list xap) {
       len += out->printer(out, fmt, 1);
       fmt++;
     } else if (fmt[0] == '%') {
-      char buf[20];
+      char buf[21];
       size_t skip = 2;
 
       if (fmt[1] == 'l' && fmt[2] == 'l' && (fmt[3] == 'd' || fmt[3] == 'u')) {
         int64_t val = va_arg(ap, int64_t);
-        const char *fmt2 = fmt[3] == 'u' ? UINT64_FMT : INT64_FMT;
+        const char *fmt2 = fmt[3] == 'u' ? "%" UINT64_FMT : "%" INT64_FMT;
         snprintf(buf, sizeof(buf), fmt2, val);
         len += out->printer(out, buf, strlen(buf));
+        skip += 2;
+      } else if (fmt[1] == 'z' && fmt[2] == 'u') {
+        size_t val = va_arg(ap, size_t);
+        snprintf(buf, sizeof(buf), "%" SIZE_T_FMT, val);
+        len += out->printer(out, buf, strlen(buf));
+        skip += 1;
       } else if (fmt[1] == 'M') {
         json_printf_callback_t f = va_arg(ap, json_printf_callback_t);
         len += f(out, &ap);
@@ -525,14 +548,51 @@ int json_vprintf(struct json_out *out, const char *fmt, va_list xap) {
           len += out->printer(out, quote, 1);
         }
       } else {
-        const char *end_of_format_specifier = "sdfFgGlhu.-0123456789";
+        const char *end_of_format_specifier = "sdfFgGlhuI.-0123456789";
         size_t n = strspn(fmt + 1, end_of_format_specifier);
         char fmt2[20];
+        va_list sub_ap;
         strncpy(fmt2, fmt, n + 1 > sizeof(fmt2) ? sizeof(fmt2) : n + 1);
         fmt2[n + 1] = '\0';
-        vsnprintf(buf, sizeof(buf), fmt2, ap);
+
+        /*
+         * we delegate printing to the system printf.
+         * The goal here is to delegate all modifiers parsing to the system
+         * printf, as you can see below we still have to parse the format types.
+         */
+        va_copy(sub_ap, ap);
+        vsnprintf(buf, sizeof(buf), fmt2, sub_ap);
+
+        /*
+         * however we need to parse the type ourselves in order to avance
+         * the va_list by the correct amount; there is no portable way to
+         * inherit the advancement made by vprintf.
+         * 32-bit (linux or windows) passes va_list by value.
+         */
+        if ((n + 1 == strlen("%" PRId64) && strcmp(fmt2, "%" PRId64) == 0) ||
+            (n + 1 == strlen("%" PRIu64) && strcmp(fmt2, "%" PRIu64) == 0)) {
+          (void) va_arg(ap, int64_t);
+          skip += strlen(PRIu64) - 1;
+        } else {
+          switch (fmt2[n]) {
+            case 'u':
+            case 'd':
+              (void) va_arg(ap, int);
+              break;
+            case 'g':
+            case 'f':
+              (void) va_arg(ap, double);
+              break;
+            case 'p':
+              (void) va_arg(ap, void *);
+              break;
+            default:
+              /* many types are promoted to int */
+              (void) va_arg(ap, int);
+          }
+        }
+
         len += out->printer(out, buf, strlen(buf));
-        CONSUME_ARG(ap);
         skip = n + 1;
       }
       fmt += skip;
@@ -585,3 +645,23 @@ int json_printf_array(struct json_out *out, va_list *ap) {
   len += json_printf(out, "]", 1);
   return len;
 }
+
+#ifdef _WIN32
+int cs_win_vsnprintf(char *str, size_t size, const char *format, va_list ap) {
+  int res = _vsnprintf(str, size, format, ap);
+  va_end(ap);
+  if (res >= size) {
+    str[size - 1] = '\0';
+  }
+  return res;
+}
+
+int cs_win_snprintf(char *str, size_t size, const char *format, ...) {
+  int res;
+  va_list ap;
+  va_start(ap, format);
+  res = vsnprintf(str, size, format, ap);
+  va_end(ap);
+  return res;
+}
+#endif /* _WIN32 */

+ 62 - 0
unit_test.c

@@ -65,6 +65,7 @@ static int cmp_token(const struct json_token *tok, const char *str,
 static const char *test_errors(void) {
   struct json_token ar[100];
   int size = ARRAY_SIZE(ar);
+  /* clang-format off */
   static const char *invalid_tests[] = {
       "1",        "a:3",           "\x01",         "{:",
       " { 1",     "{a:\"\n\"}",    "{a:1x}",       "{a:1e}",
@@ -90,6 +91,7 @@ static const char *test_errors(void) {
                                            "{a:nul",
                                            "{a:null",
                                            NULL};
+  /* clang-format on */
   static const struct {
     const char *str;
     int expected_len;
@@ -241,6 +243,44 @@ static const char *test_realloc(void) {
 static const char *test_json_printf(void) {
   char buf[200] = "";
 
+  {
+    struct json_out out = JSON_OUT_BUF(buf, sizeof(buf));
+    const char *result = "42";
+    json_printf(&out, "%d", 42);
+    ASSERT(strcmp(buf, result) == 0);
+  }
+
+  /* platform specific compatibility where it matters */
+  {
+    struct json_out out = JSON_OUT_BUF(buf, sizeof(buf));
+    const char *result = "16045690985373621933 42";
+    json_printf(&out, "%" UINT64_FMT " %d", 0xdeadbeeffee1deadUL, 42);
+    ASSERT(strcmp(buf, result) == 0);
+  }
+  {
+    struct json_out out = JSON_OUT_BUF(buf, sizeof(buf));
+    const char *result = "12 42";
+    size_t foo = 12;
+    json_printf(&out, "%" SIZE_T_FMT " %d", foo, 42);
+    ASSERT(strcmp(buf, result) == 0);
+  }
+
+  /* people live in the future today, %llu works even on recent windozes */
+  {
+    struct json_out out = JSON_OUT_BUF(buf, sizeof(buf));
+    const char *result = "16045690985373621933 42";
+    json_printf(&out, "%llu %d", 0xdeadbeeffee1deadUL, 42);
+    ASSERT(strcmp(buf, result) == 0);
+  }
+
+  {
+    struct json_out out = JSON_OUT_BUF(buf, sizeof(buf));
+    const char *result = "12 42";
+    size_t foo = 12;
+    json_printf(&out, "%zu %d", foo, 42);
+    ASSERT(strcmp(buf, result) == 0);
+  }
+
   {
     struct json_out out = JSON_OUT_BUF(buf, sizeof(buf));
     const char *result = "{\"foo\": 123, \"x\": [false, true], \"y\": \"hi\"}";
@@ -302,12 +342,34 @@ static const char *test_json_printf(void) {
   return NULL;
 }
 
+static const char *test_system() {
+  char buf[2020];
+  uint64_t u = 0xdeadbeeffee1dead;
+  int64_t d = (int64_t) u;
+  int res;
+
+  snprintf(buf, sizeof(buf), "%" UINT64_FMT, u);
+  ASSERT(strcmp(buf, "16045690985373621933") == 0);
+
+  snprintf(buf, sizeof(buf), "%" INT64_FMT, d);
+  ASSERT(strcmp(buf, "-2401053088335929683") == 0);
+
+  res = snprintf(buf, 3, "foo");
+  ASSERT(res == 3);
+  ASSERT(buf[0] == 'f');
+  ASSERT(buf[1] == 'o');
+  ASSERT(buf[2] == '\0');
+
+  return NULL;
+}
+
 static const char *run_all_tests(void) {
   RUN_TEST(test_errors);
   RUN_TEST(test_config);
   RUN_TEST(test_nested);
   RUN_TEST(test_realloc);
   RUN_TEST(test_json_printf);
+  RUN_TEST(test_system);
   return NULL;
 }