Leave printf format parsing to glibc if possible.

Call glibc's parse_printf_format() to gather information about
argument types.  This automatically handles positional parameters and
user-defined printf formats for ordinary parameter types; it doesn't
handle user-defined printf parameter types (such as defined by libdfp
for decimal floating-point).

Also add a (non-comprehensive) test.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Gergely Nagy <algernon@balabit.hu>

4 files changed, 134 insertions, 8 deletions

diff --git a/configure.ac b/configure.ac
index 749534f..7500e00 100644
--- a/configure.ac
+++ b/configure.ac
@@ -40,7 +40,7 @@ AC_CHECK_HEADERS([syslog.h dlfcn.h limits.h wchar.h])
 
 dnl ***************************************************************************
 dnl Checks for libraries
-AC_CHECK_FUNCS([gethostname strdup memset __syslog_chk])
+AC_CHECK_FUNCS([gethostname strdup memset __syslog_chk parse_printf_format])
 
 dnl The dlopen() function is in the C library for *BSD and in
 dnl libdl on GLIBC-based systems
diff --git a/lib/umberlog.c b/lib/umberlog.c
index b3fadbc..9266c76 100644
--- a/lib/umberlog.c
+++ b/lib/umberlog.c
@@ -28,6 +28,7 @@
 #define _GNU_SOURCE 1
 #define SYSLOG_NAMES 1
 
+#include "config.h"
 #include <stdarg.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -42,8 +43,10 @@
 #include <time.h>
 #include <errno.h>
 #include <wchar.h>
+#ifdef HAVE_PARSE_PRINTF_FORMAT
+#include <printf.h>
+#endif
 
-#include "config.h"
 #include "umberlog.h"
 #include "buffer.h"
 
@@ -177,8 +180,88 @@ _get_hostname (void)
   return ul_sys_settings.hostname;
 }
 
-static void
-_ul_va_spin (const char *fmt, va_list *pap)
+#ifdef HAVE_PARSE_PRINTF_FORMAT
+
+#define _ul_va_spin _ul_va_spin_glibc
+
+static int
+_ul_va_spin_glibc (const char *fmt, va_list *pap)
+{
+  size_t num_args, i;
+  int *types;
+
+  num_args = parse_printf_format (fmt, 0, NULL);
+  types = malloc (num_args * sizeof (*types));
+  if (types == NULL)
+    goto err;
+  if (parse_printf_format (fmt, num_args, types) != num_args)
+    goto err; /* Should never happen */
+
+  for (i = 0; i < num_args; i++)
+    {
+      switch (types[i])
+        {
+        case PA_CHAR:
+        case PA_INT | PA_FLAG_SHORT:
+        case PA_INT:
+          (void)va_arg (*pap, int);
+          break;
+        case PA_INT | PA_FLAG_LONG:
+          (void)va_arg (*pap, long int);
+          break;
+        case PA_INT | PA_FLAG_LONG_LONG:
+          (void)va_arg (*pap, long long int);
+          break;
+
+        case PA_WCHAR:
+          (void)va_arg (*pap, wint_t);
+          break;
+
+        case PA_STRING:
+          (void)va_arg (*pap, char *);
+          break;
+
+        case PA_WSTRING:
+          (void)va_arg (*pap, wchar_t *);
+          break;
+
+        case PA_POINTER:
+          (void)va_arg (*pap, void *);
+          break;
+
+        case PA_FLOAT:
+        case PA_DOUBLE:
+          (void)va_arg (*pap, double);
+          break;
+        case PA_DOUBLE | PA_FLAG_LONG_DOUBLE:
+          (void)va_arg (*pap, long double);
+          break;
+
+        default:
+          if ((types[i] & PA_FLAG_PTR) != 0)
+            {
+              (void)va_arg (*pap, void *);
+              break;
+            }
+          /* Unknown user-defined parameter type.  Can we log that this
+             happened? */
+          goto err;
+        }
+    }
+
+  free (types);
+  return 0;
+
+ err:
+  free (types);
+  return -1;
+}
+#else /* !HAVE_PARSE_PRINTF_FORMAT */
+
+#define _ul_va_spin _ul_va_spin_legacy
+
+static int
+_ul_va_spin_legacy (const char *fmt, va_list *pap)
 {
   size_t i;
 
@@ -293,7 +376,9 @@ _ul_va_spin (const char *fmt, va_list *pap)
             }
         }
     }
+  return 0;
 }
+#endif /* !HAVE_PARSE_PRINTF_FORMAT */
 
 /* Return a newly allocated string.
    On failure, NULL is returned and it is undefined what PAP points to. */
@@ -314,7 +399,11 @@ _ul_vasprintf_and_advance (const char *fmt, va_list *pap)
   if (res == NULL)
     return NULL;
 
-  _ul_va_spin (fmt, pap);
+  if (_ul_va_spin (fmt, pap) != 0)
+    {
+      free (res);
+      return NULL;
+    }
   return res;
 }
 
diff --git a/lib/umberlog.rst b/lib/umberlog.rst
index 704a5e3..9883de4 100644
--- a/lib/umberlog.rst
+++ b/lib/umberlog.rst
@@ -7,7 +7,7 @@ CEE-enhanced syslog message generation
 --------------------------------------
 
 :Author: Gergely Nagy <algernon@balabit.hu>
-:Date: 2012-04-28
+:Date: 2012-07-19
 :Manual section: 3
 :Manual group: CEE-enhanced syslog Manual
 
@@ -57,8 +57,10 @@ the emitted message. After the *msg_format* format string, and any
 other parameters it refers to, there must be a NULL-terminated list of
 *key*, *value format*, *format parameters*. Each of these pairs,
 constructed from the *key* and the **printf(3)**-style *value format*
-will be added to the generated message.  Note that position specifiers
-(e.g. **%2$**) are not currently supported.
+will be added to the generated message.
+
+Note that user-defined printf types defined by
+**register_printf_type()** are not supported.
 
 **ul_format()** and **ul_vformat()** do the same as the syslog
 variants above, except the formatted payload is not sent to syslog,
diff --git a/t/test_umberlog.c b/t/test_umberlog.c
index 340ea43..69d1a16 100644
--- a/t/test_umberlog.c
+++ b/t/test_umberlog.c
@@ -1,6 +1,7 @@
 #define _GNU_SOURCE 1
 
 #include "umberlog.h"
+#include "config.h"
 #include <json.h>
 #include <assert.h>
 #include <string.h>
@@ -262,6 +263,37 @@ START_TEST (test_closelog)
 }
 END_TEST
 
+#ifdef HAVE_PARSE_PRINTF_FORMAT
+START_TEST (test_positional_params)
+{
+  char *msg;
+  struct json_object *jo;
+
+  openlog ("umberlog/test_positional_params", LOG_UL_NOTIME, LOG_LOCAL0);
+
+#define COMPLEX_FORMAT \
+  "%3$*5$.*2$hhd , %1$Lf , %4$.3s , %4$s", 1.0L, 5, (char)100, "prefix", -8
+#define COMPLEX_RESULT "00100    , 1.000000 , pre , prefix"
+  msg = ul_format (LOG_DEBUG, COMPLEX_FORMAT,
+                   "simple1", "value1",
+                   "complex", COMPLEX_FORMAT,
+                   "simple2", "value2",
+                   NULL);
+  jo = parse_msg (msg);
+  free (msg);
+
+  verify_value (jo, "msg", COMPLEX_RESULT);
+  verify_value (jo, "simple1", "value1");
+  verify_value (jo, "complex", COMPLEX_RESULT);
+  verify_value (jo, "simple2", "value2");
+
+  json_object_put (jo);
+
+  closelog ();
+}
+END_TEST
+#endif
+
 int
 main (void)
 {
@@ -278,6 +310,9 @@ main (void)
   tcase_add_test (ft, test_additional_fields);
   tcase_add_test (ft, test_discover_priority);
   tcase_add_test (ft, test_no_timestamp);
+#ifdef HAVE_PARSE_PRINTF_FORMAT
+  tcase_add_test (ft, test_positional_params);
+#endif
   suite_add_tcase (s, ft);
 
   bt = tcase_create ("Bug tests");