Properly handle va_lists, by copying.

Up until now, the library abused that passing va_list around without
copying works on various architectures. On others, that does not.

To remedy the issue, the library now parses format strings itself, and
copies va_list. This makes it work on all architectures, independent
on the internal implementation of va_list, and preserves both API and
ABI.

Signed-off-by: Gergely Nagy <algernon@balabit.hu>

3 files changed, 117 insertions, 6 deletions

diff --git a/NEWS b/NEWS
index d59eb05..149d550 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,17 @@
 #+STARTUP: indent showall                       -*- org -*-
 
+* 0.1.2 - <2012-04-08 Sun>
+
+** Portability fixes
+
+Up until now, the library abused that passing va_list around without
+copying works on various architectures. On others, that does not.
+
+To remedy the issue, the library now parses format strings itself, and
+copies va_list. This makes it work on all architectures, independent
+on the internal implementation of va_list, and preserves both API and
+ABI.
+
 * 0.1.1 - <2012-04-02 Mon>
 
 ** Override __syslog_chk, even when compiled without fortification
diff --git a/TODO.org b/TODO.org
index 147954c..21843c8 100644
--- a/TODO.org
+++ b/TODO.org
@@ -84,8 +84,14 @@ CLOSED: [2012-03-20 Tue 12:29]
 I believe the best way to test would be to re-parse the JSON, extract
 the fields, and verify they're of the expected value and type.
 
-* TODO Bugfixes
-** TODO Missing va_copy() when passing around varargs
+* DONE Bugfixes
+CLOSED: [2012-04-08 Sun 10:59]
+** DONE Missing va_copy() when passing around varargs
+CLOSED: [2012-04-08 Sun 10:58]
+- CLOSING NOTE [2012-04-08 Sun 10:58] \\
+  Ended up writing a format string parsing & spinning macro. This way we
+  can keep the API and the ABI stable too at the cost of some
+  performance (which isn't the strong point of the library anyway).
 The way we pass around varargs is unsafe, and causes crashes on a few
 systems where we would need to copy the thing around.
 
diff --git a/lib/umberlog.c b/lib/umberlog.c
index ac810f8..78697c6 100644
--- a/lib/umberlog.c
+++ b/lib/umberlog.c
@@ -40,6 +40,7 @@
 #include <limits.h>
 #include <time.h>
 #include <errno.h>
+#include <wchar.h>
 
 #include "config.h"
 #include "umberlog.h"
@@ -156,6 +157,80 @@ _get_hostname (void)
   return ul_sys_settings.hostname;
 }
 
+#define _ul_va_spin(fmt,ap)                             \
+  {                                                     \
+    size_t i;                                           \
+                                                        \
+    for (i = 0; i < strlen (fmt); i++)                  \
+      {                                                 \
+        int eof = 0;                                    \
+                                                        \
+        if (fmt[i] != '%')                              \
+          continue;                                     \
+        i++;                                            \
+        while (eof != 1)                                \
+          {                                             \
+            switch (fmt[i])                             \
+              {                                         \
+              case 'd':                                 \
+              case 'i':                                 \
+              case 'o':                                 \
+              case 'u':                                 \
+              case 'x':                                 \
+              case 'X':                                 \
+                if (fmt[i - 1] == 'l')                  \
+                  {                                     \
+                    if (i - 2 > 0 && fmt[i - 2] == 'l') \
+                      (void)va_arg (ap, long long int); \
+                    else                                \
+                      (void)va_arg (ap, long int);      \
+                  }                                     \
+                else                                    \
+                  (void)va_arg (ap, int);               \
+                eof = 1;                                \
+                break;                                  \
+              case 'e':                                 \
+              case 'E':                                 \
+              case 'f':                                 \
+              case 'F':                                 \
+              case 'g':                                 \
+              case 'G':                                 \
+              case 'a':                                 \
+              case 'A':                                 \
+                if (fmt[i - 1] == 'L')                  \
+                  (void)va_arg (ap, long double);       \
+                else                                    \
+                  (void)va_arg (ap, double);            \
+                eof = 1;                                \
+                break;                                  \
+              case 'c':                                 \
+                if (fmt [i - 1] == 'l')                 \
+                  (void)va_arg (ap, wint_t);            \
+                else                                    \
+                  (void)va_arg (ap, int);               \
+                eof = 1;                                \
+                break;                                  \
+              case 's':                                 \
+                if (fmt [i - 1] == 'l')                 \
+                  (void)va_arg (ap, wchar_t *);         \
+                else                                    \
+                  (void)va_arg (ap, char *);            \
+                eof = 1;                                \
+                break;                                  \
+              case 'p':                                 \
+                (void)va_arg (ap, void *);              \
+                eof = 1;                                \
+                break;                                  \
+              case '%':                                 \
+                eof = 1;                                \
+                break;                                  \
+              default:                                  \
+                i++;                                    \
+              }                                         \
+          }                                             \
+      }                                                 \
+  }
+
 static inline struct json_object *
 _ul_json_vappend (struct json_object *json, va_list ap)
 {
@@ -166,9 +241,16 @@ _ul_json_vappend (struct json_object *json, va_list ap)
       char *fmt = (char *)va_arg (ap, char *);
       char *value = NULL;
       struct json_object *jstr;
+      va_list aq;
+
+      va_copy (aq, ap);
+      if (vasprintf (&value, fmt, aq) == -1)
+        {
+          va_end (aq);
+          return NULL;
+        }
+      va_end (aq);
 
-      if (vasprintf (&value, fmt, ap) == -1)
-        return NULL;
       if (!value)
         return NULL;
 
@@ -181,7 +263,10 @@ _ul_json_vappend (struct json_object *json, va_list ap)
 
       json_object_object_add (json, key, jstr);
       free (value);
+
+      _ul_va_spin (fmt, ap);
     }
+
   return json;
 }
 
@@ -245,9 +330,15 @@ _ul_vformat (struct json_object *jo, int format_version,
 {
   char *value;
   struct json_object *jstr;
+  va_list aq;
 
-  if (vasprintf (&value, msg_format, ap) == -1)
-    return NULL;
+  va_copy (aq, ap);
+  if (vasprintf (&value, msg_format, aq) == -1)
+    {
+      va_end (aq);
+      return NULL;
+    }
+  va_end (aq);
   if (!value)
     return NULL;
 
@@ -261,6 +352,8 @@ _ul_vformat (struct json_object *jo, int format_version,
   json_object_object_add (jo, "msg", jstr);
   free (value);
 
+  _ul_va_spin (msg_format, ap);
+
   if (format_version > 0)
     jo = _ul_json_vappend (jo, ap);