It's worth noting that `os::strerror` does break the Windows build. I have a fix here: https://reviews.apache.org/r/40382/ although it's in my review chain, it actually doesn't have any dependencies.
On Thu, Nov 19, 2015 at 12:18 PM, James Peach <jor...@gmail.com> wrote: > This commit reverts MESOS-3708. Can we please restore the previous ABORT > message? > > >> On Nov 13, 2015, at 9:11 AM, bmah...@apache.org wrote: >> >> Replaced strerror calls in libprocess with os::strerror. >> >> Review: https://reviews.apache.org/r/39007 >> >> >> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo >> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ba3be3f3 >> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ba3be3f3 >> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ba3be3f3 >> >> Branch: refs/heads/master >> Commit: ba3be3f37e6574c1a9882429fdd0939fd6406c8d >> Parents: 2fe9f6b >> Author: Benjamin Bannier <benjamin.bann...@mesosphere.io> >> Authored: Fri Nov 13 17:29:12 2015 +0100 >> Committer: Benjamin Mahler <benjamin.mah...@gmail.com> >> Committed: Fri Nov 13 17:54:19 2015 +0100 >> >> ---------------------------------------------------------------------- >> 3rdparty/libprocess/src/io.cpp | 13 +++++++------ >> 3rdparty/libprocess/src/poll_socket.cpp | 7 ++++--- >> 3rdparty/libprocess/src/process.cpp | 5 +++-- >> 3rdparty/libprocess/src/profiler.cpp | 9 +++++---- >> 3rdparty/libprocess/src/subprocess.cpp | 3 ++- >> 5 files changed, 21 insertions(+), 16 deletions(-) >> ---------------------------------------------------------------------- >> >> >> http://git-wip-us.apache.org/repos/asf/mesos/blob/ba3be3f3/3rdparty/libprocess/src/io.cpp >> ---------------------------------------------------------------------- >> diff --git a/3rdparty/libprocess/src/io.cpp b/3rdparty/libprocess/src/io.cpp >> index 26686e1..1730aaa 100644 >> --- a/3rdparty/libprocess/src/io.cpp >> +++ b/3rdparty/libprocess/src/io.cpp >> @@ -24,6 +24,7 @@ >> #include <stout/lambda.hpp> >> #include <stout/nothing.hpp> >> #include <stout/os.hpp> >> +#include <stout/os/strerror.hpp> >> #include <stout/try.hpp> >> >> using std::string; >> @@ -92,7 +93,7 @@ void read( >> WeakFuture<short>(future))); >> } else { >> // Error occurred. >> - promise->fail(strerror(errno)); >> + promise->fail(os::strerror(errno)); >> } >> } else { >> promise->set(length); >> @@ -183,7 +184,7 @@ void write( >> WeakFuture<short>(future))); >> } else { >> // Error occurred. >> - promise->fail(strerror(errno)); >> + promise->fail(os::strerror(errno)); >> } >> } else { >> // TODO(benh): Retry if 'length' is 0? >> @@ -276,7 +277,7 @@ Future<size_t> peek(int fd, void* data, size_t size, >> size_t limit) >> // also make sure it's non-blocking and will close-on-exec. Start by >> // checking we've got a "valid" file descriptor before dup'ing. >> if (fd < 0) { >> - return Failure(strerror(EBADF)); >> + return Failure(os::strerror(EBADF)); >> } >> >> fd = dup(fd); >> @@ -429,7 +430,7 @@ Future<string> read(int fd) >> // also make sure it's non-blocking and will close-on-exec. Start by >> // checking we've got a "valid" file descriptor before dup'ing. >> if (fd < 0) { >> - return Failure(strerror(EBADF)); >> + return Failure(os::strerror(EBADF)); >> } >> >> fd = dup(fd); >> @@ -475,7 +476,7 @@ Future<Nothing> write(int fd, const std::string& data) >> // also make sure it's non-blocking and will close-on-exec. Start by >> // checking we've got a "valid" file descriptor before dup'ing. >> if (fd < 0) { >> - return Failure(strerror(EBADF)); >> + return Failure(os::strerror(EBADF)); >> } >> >> fd = dup(fd); >> @@ -510,7 +511,7 @@ Future<Nothing> redirect(int from, Option<int> to, >> size_t chunk) >> { >> // Make sure we've got "valid" file descriptors. >> if (from < 0 || (to.isSome() && to.get() < 0)) { >> - return Failure(strerror(EBADF)); >> + return Failure(os::strerror(EBADF)); >> } >> >> if (to.isNone()) { >> >> http://git-wip-us.apache.org/repos/asf/mesos/blob/ba3be3f3/3rdparty/libprocess/src/poll_socket.cpp >> ---------------------------------------------------------------------- >> diff --git a/3rdparty/libprocess/src/poll_socket.cpp >> b/3rdparty/libprocess/src/poll_socket.cpp >> index 85cd864..1daeb03 100644 >> --- a/3rdparty/libprocess/src/poll_socket.cpp >> +++ b/3rdparty/libprocess/src/poll_socket.cpp >> @@ -19,6 +19,7 @@ >> #include <process/socket.hpp> >> >> #include <stout/os/sendfile.hpp> >> +#include <stout/os/strerror.hpp> >> >> #include "config.hpp" >> #include "poll_socket.hpp" >> @@ -72,7 +73,7 @@ Future<Socket> accept(int fd) >> // Turn off Nagle (TCP_NODELAY) so pipelined requests don't wait. >> int on = 1; >> if (setsockopt(s, SOL_TCP, TCP_NODELAY, &on, sizeof(on)) < 0) { >> - const char* error = strerror(errno); >> + const string error = os::strerror(errno); >> VLOG(1) << "Failed to turn off the Nagle algorithm: " << error; >> os::close(s); >> return Failure( >> @@ -159,7 +160,7 @@ Future<size_t> socket_send_data(int s, const char* data, >> size_t size) >> } else if (length <= 0) { >> // Socket error or closed. >> if (length < 0) { >> - const char* error = strerror(errno); >> + const string error = os::strerror(errno); >> VLOG(1) << "Socket error while sending: " << error; >> } else { >> VLOG(1) << "Socket closed while sending"; >> @@ -195,7 +196,7 @@ Future<size_t> socket_send_file(int s, int fd, off_t >> offset, size_t size) >> } else if (length <= 0) { >> // Socket error or closed. >> if (length < 0) { >> - const char* error = strerror(errno); >> + const string error = os::strerror(errno); >> VLOG(1) << "Socket error while sending: " << error; >> } else { >> VLOG(1) << "Socket closed while sending"; >> >> http://git-wip-us.apache.org/repos/asf/mesos/blob/ba3be3f3/3rdparty/libprocess/src/process.cpp >> ---------------------------------------------------------------------- >> diff --git a/3rdparty/libprocess/src/process.cpp >> b/3rdparty/libprocess/src/process.cpp >> index 6ab4070..b45b5b1 100644 >> --- a/3rdparty/libprocess/src/process.cpp >> +++ b/3rdparty/libprocess/src/process.cpp >> @@ -89,6 +89,7 @@ >> #include <stout/numify.hpp> >> #include <stout/option.hpp> >> #include <stout/os.hpp> >> +#include <stout/os/strerror.hpp> >> #include <stout/path.hpp> >> #include <stout/strings.hpp> >> #include <stout/synchronized.hpp> >> @@ -1096,14 +1097,14 @@ bool HttpProxy::process(const Future<Response>& >> future, const Request& request) >> VLOG(1) << "Returning '404 Not Found' for path '" << path << "'"; >> socket_manager->send(NotFound(), request, socket); >> } else { >> - const char* error = strerror(errno); >> + const string error = os::strerror(errno); >> VLOG(1) << "Failed to send file at '" << path << "': " << error; >> socket_manager->send(InternalServerError(), request, socket); >> } >> } else { >> struct stat s; // Need 'struct' because of function named 'stat'. >> if (fstat(fd, &s) != 0) { >> - const char* error = strerror(errno); >> + const string error = os::strerror(errno); >> VLOG(1) << "Failed to send file at '" << path << "': " << error; >> socket_manager->send(InternalServerError(), request, socket); >> } else if (S_ISDIR(s.st_mode)) { >> >> http://git-wip-us.apache.org/repos/asf/mesos/blob/ba3be3f3/3rdparty/libprocess/src/profiler.cpp >> ---------------------------------------------------------------------- >> diff --git a/3rdparty/libprocess/src/profiler.cpp >> b/3rdparty/libprocess/src/profiler.cpp >> index 0c51556..48a6a42 100644 >> --- a/3rdparty/libprocess/src/profiler.cpp >> +++ b/3rdparty/libprocess/src/profiler.cpp >> @@ -28,6 +28,7 @@ >> #include "stout/format.hpp" >> #include "stout/option.hpp" >> #include "stout/os.hpp" >> +#include "stout/os/strerror.hpp" >> >> namespace process { >> >> @@ -84,10 +85,10 @@ Future<http::Response> Profiler::start(const >> http::Request& request) >> // >> https://groups.google.com/d/topic/google-perftools/Df10Uy4Djrg/discussion >> // NOTE: We have not tested this with libunwind > 1.0.1. >> if (!ProfilerStart(PROFILE_FILE.c_str())) { >> - std::string error = >> - strings::format("Failed to start profiler: %s", >> strerror(errno)).get(); >> - LOG(ERROR) << error; >> - return http::InternalServerError(error); >> + Try<std::string> error = >> + strings::format("Failed to start profiler: %s", os::strerror(errno)); >> + LOG(ERROR) << error.get(); >> + return http::InternalServerError(error.get()); >> } >> >> started = true; >> >> http://git-wip-us.apache.org/repos/asf/mesos/blob/ba3be3f3/3rdparty/libprocess/src/subprocess.cpp >> ---------------------------------------------------------------------- >> diff --git a/3rdparty/libprocess/src/subprocess.cpp >> b/3rdparty/libprocess/src/subprocess.cpp >> index efe0018..58d0c1f 100644 >> --- a/3rdparty/libprocess/src/subprocess.cpp >> +++ b/3rdparty/libprocess/src/subprocess.cpp >> @@ -30,6 +30,7 @@ >> #include <stout/foreach.hpp> >> #include <stout/option.hpp> >> #include <stout/os.hpp> >> +#include <stout/os/strerror.hpp> >> #include <stout/strings.hpp> >> #include <stout/try.hpp> >> #include <stout/unreachable.hpp> >> @@ -176,7 +177,7 @@ static int childMain( >> >> os::execvpe(path.c_str(), argv, envp); >> >> - ABORT("Failed to os::execvpe on path '" + path + "': " + strerror(errno)); >> + ABORT("Failed to os::execvpe in childMain: " + os::strerror(errno)); >> } >> >> >> > -- Alex Theory is the first term in the Taylor series of practice. -- Thomas M Cover (1992)
