Use vsnprintf(3) in syslog(3).

It seemed like a clever trick to use the internal log message formatting
code in syslog(3), but on reflection that means you can't (for example)
format floating point numbers. This patch switches us over to using good
old vsnprintf(3), even though that requires us to jump through a few hoops.

There's no obvious way to unit test this, so I wrote a little program and
ran that.

Bug: 14292866
Change-Id: I9c83500ba9cbb209b6f496067a91bf69434eeef5

diff --git a/libc/bionic/libc_logging.cpp b/libc/bionic/libc_logging.cpp
index 1fb8a84..e656a12 100644 (file)
--- a/libc/bionic/libc_logging.cpp
+++ b/libc/bionic/libc_logging.cpp
@@ -230,9 +230,6 @@ static void SendRepeat(Out& o, char ch, int count) {
 /* Perform formatted output to an output target 'o' */
 template <typename Out>
 static void out_vformat(Out& o, const char* format, va_list args) {
-#if 0
-    int caller_errno = errno;
-#endif
     int nn = 0;
 
     for (;;) {
@@ -381,11 +378,6 @@ static void out_vformat(Out& o, const char* format, va_list args) {
         } else if (c == '%') {
             buffer[0] = '%';
             buffer[1] = '\0';
-#if 0
-        } else if (c == 'm') {
-            // syslog-like %m for strerror(errno).
-            str = strerror(caller_errno);
-#endif
         } else {
             __assert(__FILE__, __LINE__, "conversion specifier unsupported");
         }

diff --git a/libc/bionic/syslog.cpp b/libc/bionic/syslog.cpp
index d3b1b1e..d8b8b19 100644 (file)
--- a/libc/bionic/syslog.cpp
+++ b/libc/bionic/syslog.cpp
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <errno.h>
 #include <stdlib.h>
 #include <syslog.h>
 
@@ -46,10 +47,9 @@ void syslog(int priority, const char* fmt, ...) {
   va_end(args);
 }
 
-void vsyslog(int /*priority*/, const char* /*fmt*/, va_list /*args*/) {
-// HACK to avoid lock up on certain devices. This will be reverted when
-// that is fixed.
-#if 0
+void vsyslog(int priority, const char* fmt, va_list args) {
+  int caller_errno = errno;
+
   // Check whether we're supposed to be logging messages of this priority.
   if ((syslog_priority_mask & LOG_MASK(LOG_PRI(priority))) == 0) {
     return;
@@ -74,6 +74,48 @@ void vsyslog(int /*priority*/, const char* /*fmt*/, va_list /*args*/) {
     android_log_priority = ANDROID_LOG_DEBUG;
   }
 
-  __libc_format_log_va_list(android_log_priority, log_tag, fmt, args);
-#endif
+  // glibc's printf family support %m directly, but our BSD-based one doesn't.
+  // If the format string seems to contain "%m", rewrite it.
+  const char* log_fmt = fmt;
+  if (strstr(fmt, "%m") != NULL) {
+    size_t dst_len = 1024;
+    char* dst = reinterpret_cast<char*>(malloc(dst_len));
+    log_fmt = dst;
+
+    const char* src = fmt;
+    for (; dst_len > 0 && *src != '\0'; ++src) {
+      if (*src == '%' && *(src + 1) == 'm') {
+        // Expand %m.
+        size_t n = strlcpy(dst, strerror(caller_errno), dst_len);
+        if (n >= dst_len) {
+          n = dst_len;
+        }
+        dst += n;
+        dst_len -= n;
+        ++src;
+      } else if (*src == '%' && *(src + 1) == '%') {
+        // We need to copy pairs of '%'s so the %m test works.
+        if (dst_len <= 2) {
+          break;
+        }
+        *dst++ = '%'; --dst_len;
+        *dst++ = '%'; --dst_len;
+        ++src;
+      } else {
+        *dst++ = *src; --dst_len;
+      }
+    }
+    *dst = '\0';
+  }
+
+  // We can't let __libc_format_log do the formatting because it doesn't support
+  // all the printf functionality.
+  char log_line[1024];
+  vsnprintf(log_line, sizeof(log_line), log_fmt, args);
+
+  if (log_fmt != fmt) {
+    free(const_cast<char*>(log_fmt));
+  }
+
+  __libc_format_log(android_log_priority, log_tag, "%s", log_line);
 }

diff --git a/tests/libc_logging_test.cpp b/tests/libc_logging_test.cpp
index ef39d1c..950161e 100644 (file)
--- a/tests/libc_logging_test.cpp
+++ b/tests/libc_logging_test.cpp
@@ -176,14 +176,3 @@ TEST(libc_logging, lld_LLONG_MIN) {
   GTEST_LOG_(INFO) << "This test does nothing.\n";
 #endif // __BIONIC__
 }
-
-TEST(libc_logging, m) {
-#if defined(__BIONIC__)
-  char buf[BUFSIZ];
-  errno = EBADF;
-  __libc_format_buffer(buf, sizeof(buf), "<%m>");
-  EXPECT_STREQ("<Bad file number>", buf);
-#else // __BIONIC__
-  GTEST_LOG_(INFO) << "This test does nothing.\n";
-#endif // __BIONIC__
-}