From 0d46c7fb36042b93baea9fec4d46717b264ed22f Mon Sep 17 00:00:00 2001 From: Mike Crowe Date: Tue, 18 Jul 2017 18:19:03 +0100 Subject: [PATCH 1/2] printf: Add support for glibc's %m format glibc's printf family of functions support '%m' to emit the error string that corresponds to the current value of errno. It has no corresponding argument. In order to take advantage of safe_strerror, PrintfFormatter::format calls format_system_error, which has been modified to not emit the separator if the message prefix is empty. 'm' needs to be added to the list of special characters in format-test.cc's check_unknown_types since it would otherwise always be valid in the format string. The AppVeyor mingw test shows that errno is being reset to zero during the processing of '%m', so it needs to be explicitly preserved. This doesn't seem to happen on Linux, MacOS, or Windows with MSVC but the preservation code does no harm. --- doc/api.rst | 4 +++- fmt/format.cc | 5 ++++- fmt/printf.h | 28 ++++++++++++++++++++++++++++ test/format-test.cc | 2 +- test/printf-test.cc | 16 ++++++++++++++++ 5 files changed, 52 insertions(+), 3 deletions(-) diff --git a/doc/api.rst b/doc/api.rst index 8dbef39e..100aabbd 100644 --- a/doc/api.rst +++ b/doc/api.rst @@ -162,7 +162,9 @@ The following functions use `printf format string syntax `_ with the POSIX extension for positional arguments. Unlike their standard counterparts, the ``fmt`` functions are type-safe and throw an exception if an -argument type doesn't match its format specification. +argument type doesn't match its format specification. They also support the +glibc '%m' extension to output the string corresponding to the error code +in errno. .. doxygenfunction:: printf(CStringRef, ArgList) diff --git a/fmt/format.cc b/fmt/format.cc index 09d2ea9f..912d5b13 100644 --- a/fmt/format.cc +++ b/fmt/format.cc @@ -385,7 +385,10 @@ FMT_FUNC void format_system_error( char *system_message = &buffer[0]; int result = safe_strerror(error_code, system_message, buffer.size()); if (result == 0) { - out << message << ": " << system_message; + if (message.size()) + out << message << ": " << system_message; + else + out << system_message; return; } if (result != ERANGE) diff --git a/fmt/printf.h b/fmt/printf.h index 30cbc49b..c757b080 100644 --- a/fmt/printf.h +++ b/fmt/printf.h @@ -12,6 +12,7 @@ #include // std::fill_n #include // std::numeric_limits +#include // errno #include "ostream.h" @@ -197,6 +198,18 @@ class WidthHandler : public ArgVisitor { return static_cast(width); } }; + +class ErrnoPreserver { + const int saved_errno_; +public: + ErrnoPreserver() : saved_errno_(errno) {} + ~ErrnoPreserver() { + errno = saved_errno_; + } + int get() const { + return saved_errno_; + } +}; } // namespace internal /** @@ -431,6 +444,21 @@ void PrintfFormatter::format(BasicCStringRef format_str) { // Parse argument index, flags and width. unsigned arg_index = parse_header(s, spec); + if (*s == 'm') { + // Something in here is changing errno in the mingw build. We + // ought to preserve it anyway, even if an exception is thrown. + internal::ErrnoPreserver errno_preserver; + + // This dance is necessary because this function is templated on + // the character type but format_system_error is not. + MemoryWriter char_writer; + format_system_error(char_writer, errno_preserver.get(), ""); + AF(writer_, spec).visit_cstring(char_writer.c_str()); + + start = ++s; + continue; + } + // Parse precision. if (*s == '.') { ++s; diff --git a/test/format-test.cc b/test/format-test.cc index 6388d5a5..3814bbb7 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -1105,7 +1105,7 @@ template void check_unknown_types( const T &value, const char *types, const char *type_name) { char format_str[BUFFER_SIZE], message[BUFFER_SIZE]; - const char *special = ".0123456789}"; + const char *special = ".0123456789}m"; for (int i = CHAR_MIN; i <= CHAR_MAX; ++i) { char c = static_cast(i); if (std::strchr(types, c) || std::strchr(special, c) || !c) continue; diff --git a/test/printf-test.cc b/test/printf-test.cc index 81a041d7..a0443838 100644 --- a/test/printf-test.cc +++ b/test/printf-test.cc @@ -51,6 +51,10 @@ std::string make_positional(fmt::StringRef format) { << "format: " << format; \ EXPECT_EQ(expected_output, fmt::sprintf(make_positional(format), arg)) +#define EXPECT_PRINTF_NO_ARG(expected_output, format) \ + EXPECT_EQ(expected_output, fmt::sprintf(format)) \ + << "format: " << format; + TEST(PrintfTest, NoArgs) { EXPECT_EQ("test", fmt::sprintf("test")); } @@ -472,6 +476,18 @@ TEST(PrintfTest, Enum) { EXPECT_PRINTF("42", "%d", A); } +TEST(PrintfTest, ErrorMessage) { + errno = ENOENT; + + EXPECT_PRINTF_NO_ARG("No such file or directory", "%m"); + EXPECT_PRINTF_NO_ARG("No such file or directory ", "%-30m"); + EXPECT_PRINTF_NO_ARG(" No such file or directory", "%30m"); + + EXPECT_PRINTF_NO_ARG(L"No such file or directory", L"%m"); + EXPECT_PRINTF_NO_ARG(L"No such file or directory ", L"%-30m"); + EXPECT_PRINTF_NO_ARG(L" No such file or directory", L"%30m"); +} + #if FMT_USE_FILE_DESCRIPTORS TEST(PrintfTest, Examples) { const char *weekday = "Thursday"; From a5231b86df5a9171978f21364c6b92ac6cbf502b Mon Sep 17 00:00:00 2001 From: Mike Crowe Date: Sat, 22 Jul 2017 14:42:37 +0100 Subject: [PATCH 2/2] printf-test: Add check that errno is preserved by fmt::sprintf Since I accidentally added code to PrintfFormatter::format that modified errno on mingw whilst adding support for '%m', it seems worth having a test to make sure that can't happen again in the future. --- test/printf-test.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/printf-test.cc b/test/printf-test.cc index a0443838..3f25670e 100644 --- a/test/printf-test.cc +++ b/test/printf-test.cc @@ -476,6 +476,21 @@ TEST(PrintfTest, Enum) { EXPECT_PRINTF("42", "%d", A); } +TEST(PrintfTest, ErrnoPreserved) { + errno = ENOENT; + EXPECT_PRINTF_NO_ARG("testing 1 2 3", "testing 1 2 3"); + EXPECT_EQ(errno, ENOENT); + + EXPECT_PRINTF("42", "%d", 42); + EXPECT_EQ(errno, ENOENT); + + EXPECT_PRINTF(" Hello", "%6s", "Hello"); + EXPECT_EQ(errno, ENOENT); + + EXPECT_PRINTF_NO_ARG("No such file or directory", "%m"); + EXPECT_EQ(errno, ENOENT); +} + TEST(PrintfTest, ErrorMessage) { errno = ENOENT;