Removed use of `fstat()` from `http.cpp` and `http_proxy.cpp`. The functions `os::stat::size()` and `os::stat::isdir()` are now overloaded for an `int_fd` type, using `fstat()` on POSIX, and the equivalent functions with a `HANDLE` on Windows. This allowed us to remove the use of `::fstat()`, which was not abstracted, and not supported on Windows without the use of a CRT integer file descriptor.
Review: https://reviews.apache.org/r/66436 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/92d340f2 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/92d340f2 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/92d340f2 Branch: refs/heads/master Commit: 92d340f2d10b78a1e761dd930f8fbd89992c14e3 Parents: cf6c332 Author: Andrew Schwartzmeyer <[email protected]> Authored: Tue Apr 3 11:20:42 2018 -0700 Committer: Andrew Schwartzmeyer <[email protected]> Committed: Tue May 1 18:36:04 2018 -0700 ---------------------------------------------------------------------- 3rdparty/libprocess/src/http.cpp | 19 ++++++------------- 3rdparty/libprocess/src/http_proxy.cpp | 27 +++++++++------------------ 2 files changed, 15 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/92d340f2/3rdparty/libprocess/src/http.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/http.cpp b/3rdparty/libprocess/src/http.cpp index 63dd2c1..09f4a0a 100644 --- a/3rdparty/libprocess/src/http.cpp +++ b/3rdparty/libprocess/src/http.cpp @@ -1549,23 +1549,16 @@ Future<Nothing> sendfile( return send(socket, InternalServerError(body), request); } - struct stat s; // Need 'struct' because of function named 'stat'. - // We don't bother introducing a `os::fstat` since this is only - // one of two places where we use `fstat` in the entire codebase - // as of writing this comment. -#ifdef __WINDOWS__ - if (::fstat(fd->crt(), &s) != 0) { -#else - if (::fstat(fd.get(), &s) != 0) { -#endif + const Try<Bytes> size = os::stat::size(fd.get()); + if (size.isError()) { const string body = - "Failed to fstat '" + response.path + "': " + os::strerror(errno); + "Failed to fstat '" + response.path + "': " + size.error(); // TODO(benh): VLOG(1)? // TODO(benh): Don't send error back as part of InternalServiceError? // TODO(benh): Copy headers from `response`? os::close(fd.get()); return send(socket, InternalServerError(body), request); - } else if (S_ISDIR(s.st_mode)) { + } else if (os::stat::isdir(fd.get())) { const string body = "'" + response.path + "' is a directory"; // TODO(benh): VLOG(1)? // TODO(benh): Don't send error back as part of InternalServiceError? @@ -1576,7 +1569,7 @@ Future<Nothing> sendfile( // While the user is expected to properly set a 'Content-Type' // header, we'll fill in (or overwrite) 'Content-Length' header. - response.headers["Content-Length"] = stringify(s.st_size); + response.headers["Content-Length"] = stringify(size->bytes()); // TODO(benh): If this is a TCP socket consider turning on TCP_CORK // for both sends and then turning it off. @@ -1593,7 +1586,7 @@ Future<Nothing> sendfile( }) .then([=]() mutable -> Future<Nothing> { // NOTE: the file descriptor gets closed by FileEncoder. - Encoder* encoder = new FileEncoder(fd.get(), s.st_size); + Encoder* encoder = new FileEncoder(fd.get(), size->bytes()); return send(socket, encoder) .onAny([=]() { delete encoder; http://git-wip-us.apache.org/repos/asf/mesos/blob/92d340f2/3rdparty/libprocess/src/http_proxy.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/http_proxy.cpp b/3rdparty/libprocess/src/http_proxy.cpp index 25d6379..3ac353a 100644 --- a/3rdparty/libprocess/src/http_proxy.cpp +++ b/3rdparty/libprocess/src/http_proxy.cpp @@ -161,34 +161,25 @@ bool HttpProxy::process(const Future<Response>& future, const Request& request) socket_manager->send(InternalServerError(), request, socket); } } else { - struct stat s; // Need 'struct' because of function named 'stat'. - // We don't bother introducing a `os::fstat` since this is only - // one of two places where we use `fstat` in the entire codebase - // as of writing this comment. -#ifdef __WINDOWS__ - if (::fstat(fd.crt(), &s) != 0) { -#else - if (::fstat(fd, &s) != 0) { -#endif - const string error = os::strerror(errno); - VLOG(1) << "Failed to send file at '" << path << "': " << error; + const Try<Bytes> size = os::stat::size(fd); + if (size.isError()) { + VLOG(1) << "Failed to send file at '" << path << "': " << size.error(); socket_manager->send(InternalServerError(), request, socket); - } else if (S_ISDIR(s.st_mode)) { + } else if (os::stat::isdir(fd)) { VLOG(1) << "Returning '404 Not Found' for directory '" << path << "'"; socket_manager->send(NotFound(), request, socket); } else { // While the user is expected to properly set a 'Content-Type' // header, we fill in (or overwrite) 'Content-Length' header. - stringstream out; - out << s.st_size; - response.headers["Content-Length"] = out.str(); + response.headers["Content-Length"] = stringify(size->bytes()); - if (s.st_size == 0) { + if (size.get() == 0) { socket_manager->send(response, request, socket); return true; // All done, can process next request. } - VLOG(1) << "Sending file at '" << path << "' with length " << s.st_size; + VLOG(1) << "Sending file at '" << path << "' with length " + << size.get(); // TODO(benh): Consider a way to have the socket manager turn // on TCP_CORK for both sends and then turn it off. @@ -199,7 +190,7 @@ bool HttpProxy::process(const Future<Response>& future, const Request& request) // Note the file descriptor gets closed by FileEncoder. socket_manager->send( - new FileEncoder(fd, s.st_size), + new FileEncoder(fd, size->bytes()), request.keepAlive, socket); }
