You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2018/05/02 01:38:18 UTC

[18/31] mesos git commit: Removed use of `fstat()` from `http.cpp` and `http_proxy.cpp`.

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 <an...@schwartzmeyer.com>
Authored: Tue Apr 3 11:20:42 2018 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
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);
       }