Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

[Eric Fiselier via cfe-commits]

EricWF updated this revision to Diff 60649.
EricWF added a comment.

Use named variable `strerror_buff_size` variable instead of literal `1024`.


http://reviews.llvm.org/D20903

Files:
  src/system_error.cpp
  
test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
  
test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp


Index: test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
===================================================================
--- test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
+++ test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
@@ -16,6 +16,15 @@
 #include <system_error>
 #include <cassert>
 #include <string>
+#include <cerrno>
+
+
+void test_message_leaves_errno_unchanged() {
+    errno = E2BIG; // something that message will never generate
+    const std::error_category& e_cat1 = std::system_category();
+    e_cat1.message(-1);
+    assert(errno == E2BIG);
+}
 
 int main()
 {
@@ -26,4 +35,7 @@
     e_cond = e_cat1.default_error_condition(5000);
     assert(e_cond.value() == 5000);
     assert(e_cond.category() == std::system_category());
+    {
+        test_message_leaves_errno_unchanged();
+    }
 }
Index: test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
===================================================================
--- test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
+++ test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
@@ -16,10 +16,21 @@
 #include <system_error>
 #include <cassert>
 #include <string>
+#include <cerrno>
+
+void test_message_leaves_errno_unchanged() {
+    errno = E2BIG; // something that message will never generate
+    const std::error_category& e_cat1 = std::generic_category();
+    e_cat1.message(-1);
+    assert(errno == E2BIG);
+}
 
 int main()
 {
     const std::error_category& e_cat1 = std::generic_category();
     std::string m1 = e_cat1.name();
     assert(m1 == "generic");
+    {
+        test_message_leaves_errno_unchanged();
+    }
 }
Index: src/system_error.cpp
===================================================================
--- src/system_error.cpp
+++ src/system_error.cpp
@@ -13,8 +13,13 @@
 #include "system_error"
 
 #include "include/config_elast.h"
+#include "cerrno"
 #include "cstring"
+#include "cstdio"
+#include "cstdlib"
+#include "cassert"
 #include "string"
+#include "string.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
@@ -46,10 +51,52 @@
     return *this == code.category() && code.value() == condition;
 }
 
+namespace {
+
+//  GLIBC also uses 1024 as the maximum buffer size internally.
+constexpr size_t strerror_buff_size = 1024;
+
+string do_strerror_r(int ev);
+
+#if defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC)
+// GNU Extended version
+string do_strerror_r(int ev) {
+    char buffer[strerror_buff_size];
+    char* ret = ::strerror_r(ev, buffer, strerror_buff_size);
+    return string(ret);
+}
+#else
+// POSIX version
+string do_strerror_r(int ev) {
+    char buffer[strerror_buff_size];
+    const int old_errno = errno;
+    if (::strerror_r(ev, buffer, strerror_buff_size) == -1) {
+        const int new_errno = errno;
+        errno = old_errno;
+        if (new_errno == EINVAL) {
+            std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev);
+            return string(buffer);
+        } else {
+            assert(new_errno == ERANGE);
+            // FIXME maybe? 'strerror_buff_size' is likely to exceed the
+            // maximum error size so ERANGE shouldn't be returned.
+            std::abort();
+        }
+    }
+    return string(buffer);
+}
+#endif
+
+} // end namespace
+
 string
 __do_message::message(int ev) const
 {
-    return string(strerror(ev));
+#if defined(_LIBCPP_HAS_NO_THREADS)
+    return string(::strerror(ev));
+#else
+    return do_strerror_r(ev);
+#endif
 }
 
 class _LIBCPP_HIDDEN __generic_error_category

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits