From f1e83cc34a58761fc7bc5178c6f283db85d6057a Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Fri, 25 Jul 2014 20:31:47 -0700 Subject: [PATCH] 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. (cherry-pick of b1b60c30bf321c0fc02264b953b5c16c49d34457.) Bug: 14292866 Change-Id: I9c83500ba9cbb209b6f496067a91bf69434eeef5 --- libc/bionic/libc_logging.cpp | 4 --- libc/bionic/syslog.cpp | 48 +++++++++++++++++++++++++++++++++++- tests/libc_logging_test.cpp | 11 --------- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/libc/bionic/libc_logging.cpp b/libc/bionic/libc_logging.cpp index b57e24297..d0172edaa 100644 --- a/libc/bionic/libc_logging.cpp +++ b/libc/bionic/libc_logging.cpp @@ -230,7 +230,6 @@ static void SendRepeat(Out& o, char ch, int count) { /* Perform formatted output to an output target 'o' */ template static void out_vformat(Out& o, const char* format, va_list args) { - int caller_errno = errno; int nn = 0; for (;;) { @@ -379,9 +378,6 @@ static void out_vformat(Out& o, const char* format, va_list args) { } else if (c == '%') { buffer[0] = '%'; buffer[1] = '\0'; - } else if (c == 'm') { - // syslog-like %m for strerror(errno). - str = strerror(caller_errno); } else { __assert(__FILE__, __LINE__, "conversion specifier unsupported"); } diff --git a/libc/bionic/syslog.cpp b/libc/bionic/syslog.cpp index 29f892a20..d8b8b190d 100644 --- a/libc/bionic/syslog.cpp +++ b/libc/bionic/syslog.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -47,6 +48,8 @@ void syslog(int priority, const char* fmt, ...) { } 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; @@ -71,5 +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); + // 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(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(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 ef39d1c03..950161e78 100644 --- 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("", buf); -#else // __BIONIC__ - GTEST_LOG_(INFO) << "This test does nothing.\n"; -#endif // __BIONIC__ -}