You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mp...@apache.org on 2016/11/17 23:52:11 UTC

[1/2] mesos git commit: Support explicit error codes in `ErrnoError` and `SocketError`.

Repository: mesos
Updated Branches:
  refs/heads/master 45db35571 -> 86303ed30


Support explicit error codes in `ErrnoError` and `SocketError`.

Pass the error code down to the `Error` object explicitly, rather than
setting the per-thread error and using it implicitly.

Review: https://reviews.apache.org/r/53474/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/18ec7af0
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/18ec7af0
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/18ec7af0

Branch: refs/heads/master
Commit: 18ec7af0785b27aa8e1f1ebf5fa0f52f58d5b13b
Parents: 45db355
Author: James Peach <jp...@apache.org>
Authored: Thu Nov 17 13:35:20 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Thu Nov 17 13:37:40 2016 -0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/errorbase.hpp      | 11 +++++---
 3rdparty/stout/include/stout/os/posix/rmdir.hpp |  3 +--
 3rdparty/stout/include/stout/os/windows/su.hpp  |  3 +--
 3rdparty/stout/include/stout/windows/error.hpp  | 14 ++++++++--
 3rdparty/stout/include/stout/windows/fs.hpp     |  2 +-
 3rdparty/stout/include/stout/windows/os.hpp     |  4 +--
 3rdparty/stout/tests/error_tests.cpp            | 28 ++++++++++++++++++++
 7 files changed, 53 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/18ec7af0/3rdparty/stout/include/stout/errorbase.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/errorbase.hpp b/3rdparty/stout/include/stout/errorbase.hpp
index 96d395d..2118b8e 100644
--- a/3rdparty/stout/include/stout/errorbase.hpp
+++ b/3rdparty/stout/include/stout/errorbase.hpp
@@ -44,10 +44,15 @@ public:
 class ErrnoError : public Error
 {
 public:
-  ErrnoError() : Error(os::strerror(errno)), code(errno) {}
+  ErrnoError() : ErrnoError(errno) {}
 
-  ErrnoError(const std::string& message)
-    : Error(message + ": " + os::strerror(errno)), code(errno) {}
+  explicit ErrnoError(int _code) : Error(os::strerror(_code)), code(_code) {}
+
+  explicit ErrnoError(const std::string& message)
+    : ErrnoError(errno, message) {}
+
+  ErrnoError(int _code, const std::string& message)
+    : Error(message + ": " + os::strerror(_code)), code(_code) {}
 
   const int code;
 };

http://git-wip-us.apache.org/repos/asf/mesos/blob/18ec7af0/3rdparty/stout/include/stout/os/posix/rmdir.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/posix/rmdir.hpp b/3rdparty/stout/include/stout/os/posix/rmdir.hpp
index 5657b5a..2427bd2 100644
--- a/3rdparty/stout/include/stout/os/posix/rmdir.hpp
+++ b/3rdparty/stout/include/stout/os/posix/rmdir.hpp
@@ -54,8 +54,7 @@ inline Try<Nothing> rmdir(
     // exist. We manually induce an error here to indicate that we can't remove
     // a directory that does not exist.
     if (!os::exists(directory)) {
-      errno = ENOENT;
-      return ErrnoError();
+      return ErrnoError(ENOENT);
     }
 
     char* paths[] = {const_cast<char*>(directory.c_str()), nullptr};

http://git-wip-us.apache.org/repos/asf/mesos/blob/18ec7af0/3rdparty/stout/include/stout/os/windows/su.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/su.hpp b/3rdparty/stout/include/stout/os/windows/su.hpp
index 777140e..1bb7096 100644
--- a/3rdparty/stout/include/stout/os/windows/su.hpp
+++ b/3rdparty/stout/include/stout/os/windows/su.hpp
@@ -52,8 +52,7 @@ inline Result<gid_t> getgid(const Option<std::string>& user = None()) = delete;
 
 inline Result<std::string> user(Option<uid_t> uid = None())
 {
-  SetLastError(ERROR_NOT_SUPPORTED);
-  return WindowsError();
+  return WindowsError(ERROR_NOT_SUPPORTED);
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/18ec7af0/3rdparty/stout/include/stout/windows/error.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/windows/error.hpp b/3rdparty/stout/include/stout/windows/error.hpp
index f296b82..f8a0825 100644
--- a/3rdparty/stout/include/stout/windows/error.hpp
+++ b/3rdparty/stout/include/stout/windows/error.hpp
@@ -26,8 +26,8 @@
 // standard libraries, it wraps an error coming from the Windows APIs.
 class WindowsErrorBase : public Error
 {
-public:
-  WindowsErrorBase(DWORD _code)
+protected:
+  explicit WindowsErrorBase(DWORD _code)
     : Error(get_last_error_as_string(_code)), code(_code) {}
 
   WindowsErrorBase(DWORD _code, const std::string& message)
@@ -105,8 +105,13 @@ class WindowsError : public WindowsErrorBase {
 public:
   WindowsError() : WindowsErrorBase(::GetLastError()) {}
 
+  explicit WindowsError(DWORD _code) : WindowsErrorBase(_code) {}
+
   WindowsError(const std::string& message)
     : WindowsErrorBase(::GetLastError(), message) {}
+
+  WindowsError(DWORD _code, const std::string& message)
+    : WindowsErrorBase(_code, message) {}
 };
 
 
@@ -114,8 +119,13 @@ class WindowsSocketError : public WindowsErrorBase {
 public:
   WindowsSocketError() : WindowsErrorBase(::WSAGetLastError()) {}
 
+  explicit WindowsSocketError(DWORD _code) : WindowsErrorBase(_code) {}
+
   WindowsSocketError(const std::string& message)
     : WindowsErrorBase(::WSAGetLastError(), message) {}
+
+  WindowsSocketError(DWORD _code, const std::string& message)
+    : WindowsErrorBase(_code, message) {}
 };
 
 #endif // __STOUT_WINDOWS_ERROR_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/18ec7af0/3rdparty/stout/include/stout/windows/fs.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/windows/fs.hpp b/3rdparty/stout/include/stout/windows/fs.hpp
index 06c46db..7c3413b 100644
--- a/3rdparty/stout/include/stout/windows/fs.hpp
+++ b/3rdparty/stout/include/stout/windows/fs.hpp
@@ -119,8 +119,8 @@ inline Try<std::list<std::string>> list(const std::string& pattern)
   ::FindClose(search_handle);
 
   if (error != ERROR_NO_MORE_FILES) {
-    ::SetLastError(error);
     return WindowsError(
+        error,
         "'fs::list': 'FindNextFile' failed when searching for files with "
         "'pattern '" + pattern + "'");
   }

http://git-wip-us.apache.org/repos/asf/mesos/blob/18ec7af0/3rdparty/stout/include/stout/windows/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/windows/os.hpp b/3rdparty/stout/include/stout/windows/os.hpp
index 7ca0b5d..de9b04a 100644
--- a/3rdparty/stout/include/stout/windows/os.hpp
+++ b/3rdparty/stout/include/stout/windows/os.hpp
@@ -425,8 +425,8 @@ inline Try<Load> loadavg()
   // No Windows equivalent, return an error until there is a need. We can
   // construct an approximation of this function by periodically polling
   // `GetSystemTimes` and using a sliding window of statistics.
-  return WindowsErrorBase(ERROR_NOT_SUPPORTED,
-                          "Failed to determine system load averages");
+  return WindowsError(ERROR_NOT_SUPPORTED,
+                      "Failed to determine system load averages");
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/18ec7af0/3rdparty/stout/tests/error_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/error_tests.cpp b/3rdparty/stout/tests/error_tests.cpp
index 25b5014..5d5f6aa 100644
--- a/3rdparty/stout/tests/error_tests.cpp
+++ b/3rdparty/stout/tests/error_tests.cpp
@@ -24,6 +24,7 @@
 
 using std::string;
 
+using testing::StartsWith;
 
 Error error1()
 {
@@ -71,3 +72,30 @@ TEST(ErrorTest, Test)
   r = error5(error1());
   EXPECT_ERROR(r);
 }
+
+
+TEST(ErrorTest, Errno)
+{
+#ifdef __WINDOWS__
+  DWORD einval = ERROR_INVALID_HANDLE;
+#else
+  int einval = EINVAL;
+#endif
+
+#ifdef __WINDOWS__
+  DWORD notsock = WSAENOTSOCK;
+#else
+  int notsock = ENOTSOCK;
+#endif
+
+  EXPECT_EQ(einval, ErrnoError(einval).code);
+  EXPECT_EQ(notsock, SocketError(notsock).code);
+
+  EXPECT_EQ(einval, ErrnoError(einval, "errno error").code);
+  EXPECT_THAT(
+    ErrnoError(einval, "errno error").message, StartsWith("errno error"));
+
+  EXPECT_EQ(notsock, SocketError(notsock, "socket error").code);
+  EXPECT_THAT(
+    SocketError(notsock, "socket error").message, StartsWith("socket error"));
+}


[2/2] mesos git commit: Use explicit error codes in `ErrnoError` and `SocketError`.

Posted by mp...@apache.org.
Use explicit error codes in `ErrnoError` and `SocketError`.

Pass the error code down to the `Error` object explicitly, rather than
setting the per-thread error and using it implicitly.

Review: https://reviews.apache.org/r/53475/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/86303ed3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/86303ed3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/86303ed3

Branch: refs/heads/master
Commit: 86303ed30832ec919bd51d125dc330d969f58b8b
Parents: 18ec7af
Author: James Peach <jp...@apache.org>
Authored: Thu Nov 17 13:35:32 2016 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Thu Nov 17 13:37:47 2016 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/src/libevent_ssl_socket.cpp |  3 +--
 3rdparty/libprocess/src/poll_socket.cpp         | 12 +-----------
 2 files changed, 2 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/86303ed3/3rdparty/libprocess/src/libevent_ssl_socket.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.cpp b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
index 21f878e..5c0929d 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -163,8 +163,7 @@ Try<Nothing> LibeventSSLSocketImpl::shutdown()
       CHECK(recv_request.get() == nullptr);
       CHECK(send_request.get() == nullptr);
 
-      errno = ENOTCONN;
-      return ErrnoError();
+      return ErrnoError(ENOTCONN);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/86303ed3/3rdparty/libprocess/src/poll_socket.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/poll_socket.cpp b/3rdparty/libprocess/src/poll_socket.cpp
index f0ee149..eb7b487 100644
--- a/3rdparty/libprocess/src/poll_socket.cpp
+++ b/3rdparty/libprocess/src/poll_socket.cpp
@@ -131,17 +131,7 @@ Future<Nothing> connect(const Socket& socket, const Address& to)
   }
 
   if (opt != 0) {
-    // Make the error visible to the `SocketError` constructor by pushing
-    // it into the global per-thread error. MESOS-6520 which should
-    // allow us to pass the error code directly into `SocketError`.
-
-#ifdef __WINDOWS__
-    ::WSASetLastError(opt);
-#else
-    errno = opt;
-#endif
-
-    return Failure(SocketError("Failed to connect to " + stringify(to)));
+    return Failure(SocketError(opt, "Failed to connect to " + stringify(to)));
   }
 
   return Nothing();