syscall: use Api::SysCallResult in buffer impl

include/envoy/buffer/BUILD

@@ -11,4 +11,5 @@ envoy_package()
 envoy_cc_library(
     name = "buffer_interface",
     hdrs = ["buffer.h"],
+    deps = ["//include/envoy/api:os_sys_calls_interface"],
 )

include/envoy/buffer/buffer.h

@@ -4,8 +4,8 @@
 #include <functional>
 #include <memory>
 #include <string>
-#include <tuple>
 
+#include "envoy/api/os_sys_calls.h"
 #include "envoy/common/pure.h"
 
 namespace Envoy {
@@ -143,11 +143,10 @@ class Instance {
    * Read from a file descriptor directly into the buffer.
    * @param fd supplies the descriptor to read from.
    * @param max_length supplies the maximum length to read.
-   * @return a tuple with the number of bytes read and the errno. If an error occurred, the
-   *   number of bytes read would indicate -1 and the errno would be non-zero. Otherwise, if
-   *   bytes were read, errno shouldn't be used.
+   * @return a Api::SysCallResult with rc_ = the number of bytes read if successful, or rc_ = -1
+   *   for failure. If the call is successful, errno_ shouldn't be used.
    */
-  virtual std::tuple<int, int> read(int fd, uint64_t max_length) PURE;
+  virtual Api::SysCallResult read(int fd, uint64_t max_length) PURE;
 
   /**
    * Reserve space in the buffer.
@@ -176,11 +175,10 @@ class Instance {
   /**
    * Write the buffer out to a file descriptor.
    * @param fd supplies the descriptor to write to.
-   * @return a tuple with the number of bytes written and the errno. If an error occurred, the
-   *   number of bytes written would indicate -1 and the errno would be non-zero. Otherwise, if
-   *   bytes were written, errno shouldn't be used.
+   * @return a Api::SysCallResult with rc_ = the number of bytes written if successful, or rc_ = -1
+   *   for failure. If the call is successful, errno_ shouldn't be used.
    */
-  virtual std::tuple<int, int> write(int fd) PURE;
+  virtual Api::SysCallResult write(int fd) PURE;
 };
 
 typedef std::unique_ptr<Instance> InstancePtr;

source/common/buffer/buffer_impl.cc

@@ -94,9 +94,9 @@ void OwnedImpl::move(Instance& rhs, uint64_t length) {
   static_cast<LibEventInstance&>(rhs).postProcess();
 }
 
-std::tuple<int, int> OwnedImpl::read(int fd, uint64_t max_length) {
+Api::SysCallResult OwnedImpl::read(int fd, uint64_t max_length) {
   if (max_length == 0) {
-    return std::make_tuple(0, 0);
+    return {0, 0};
   }
   constexpr uint64_t MaxSlices = 2;
   RawSlice slices[MaxSlices];
@@ -117,7 +117,7 @@ std::tuple<int, int> OwnedImpl::read(int fd, uint64_t max_length) {
   const ssize_t rc = os_syscalls.readv(fd, iov, static_cast<int>(num_slices_to_read));
   const int error = errno;
   if (rc < 0) {
-    return std::make_tuple(rc, error);
+    return {static_cast<int>(rc), error};
   }
   uint64_t num_slices_to_commit = 0;
   uint64_t bytes_to_commit = rc;
@@ -131,7 +131,7 @@ std::tuple<int, int> OwnedImpl::read(int fd, uint64_t max_length) {
   }
   ASSERT(num_slices_to_commit <= num_slices);
   commit(slices, num_slices_to_commit);
-  return std::make_tuple(rc, error);
+  return {static_cast<int>(rc), error};
 }
 
 uint64_t OwnedImpl::reserve(uint64_t length, RawSlice* iovecs, uint64_t num_iovecs) {
@@ -152,7 +152,7 @@ ssize_t OwnedImpl::search(const void* data, uint64_t size, size_t start) const {
   return result_ptr.pos;
 }
 
-std::tuple<int, int> OwnedImpl::write(int fd) {
+Api::SysCallResult OwnedImpl::write(int fd) {
   constexpr uint64_t MaxSlices = 16;
   RawSlice slices[MaxSlices];
   const uint64_t num_slices = std::min(getRawSlices(slices, MaxSlices), MaxSlices);
@@ -166,15 +166,15 @@ std::tuple<int, int> OwnedImpl::write(int fd) {
     }
   }
   if (num_slices_to_write == 0) {
-    return std::make_tuple(0, 0);
+    return {0, 0};
   }
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
   const ssize_t rc = os_syscalls.writev(fd, iov, num_slices_to_write);
   const int error = errno;
   if (rc > 0) {
     drain(static_cast<uint64_t>(rc));
   }
-  return std::make_tuple(static_cast<int>(rc), error);
+  return {static_cast<int>(rc), error};
 }
 
 OwnedImpl::OwnedImpl() : buffer_(evbuffer_new()) {}

source/common/buffer/buffer_impl.h

@@ -80,10 +80,10 @@ class OwnedImpl : public LibEventInstance {
   void* linearize(uint32_t size) override;
   void move(Instance& rhs) override;
   void move(Instance& rhs, uint64_t length) override;
-  std::tuple<int, int> read(int fd, uint64_t max_length) override;
+  Api::SysCallResult read(int fd, uint64_t max_length) override;
   uint64_t reserve(uint64_t length, RawSlice* iovecs, uint64_t num_iovecs) override;
   ssize_t search(const void* data, uint64_t size, size_t start) const override;
-  std::tuple<int, int> write(int fd) override;
+  Api::SysCallResult write(int fd) override;
   void postProcess() override {}
   std::string toString() const override;
 

source/common/buffer/watermark_buffer.cc

@@ -40,8 +40,8 @@ void WatermarkBuffer::move(Instance& rhs, uint64_t length) {
   checkHighWatermark();
 }
 
-std::tuple<int, int> WatermarkBuffer::read(int fd, uint64_t max_length) {
-  std::tuple<int, int> result = OwnedImpl::read(fd, max_length);
+Api::SysCallResult WatermarkBuffer::read(int fd, uint64_t max_length) {
+  Api::SysCallResult result = OwnedImpl::read(fd, max_length);
   checkHighWatermark();
   return result;
 }
@@ -52,8 +52,8 @@ uint64_t WatermarkBuffer::reserve(uint64_t length, RawSlice* iovecs, uint64_t nu
   return bytes_reserved;
 }
 
-std::tuple<int, int> WatermarkBuffer::write(int fd) {
-  std::tuple<int, int> result = OwnedImpl::write(fd);
+Api::SysCallResult WatermarkBuffer::write(int fd) {
+  Api::SysCallResult result = OwnedImpl::write(fd);
   checkLowWatermark();
   return result;
 }

source/common/buffer/watermark_buffer.h

@@ -28,9 +28,9 @@ class WatermarkBuffer : public OwnedImpl {
   void drain(uint64_t size) override;
   void move(Instance& rhs) override;
   void move(Instance& rhs, uint64_t length) override;
-  std::tuple<int, int> read(int fd, uint64_t max_length) override;
+  Api::SysCallResult read(int fd, uint64_t max_length) override;
   uint64_t reserve(uint64_t length, RawSlice* iovecs, uint64_t num_iovecs) override;
-  std::tuple<int, int> write(int fd) override;
+  Api::SysCallResult write(int fd) override;
   void postProcess() override { checkLowWatermark(); }
 
   void setWatermarks(uint32_t watermark) { setWatermarks(watermark / 2, watermark); }

source/common/network/raw_buffer_socket.cc

@@ -17,25 +17,22 @@ IoResult RawBufferSocket::doRead(Buffer::Instance& buffer) {
   bool end_stream = false;
   do {
     // 16K read is arbitrary. TODO(mattklein123) PERF: Tune the read size.
-    std::tuple<int, int> result = buffer.read(callbacks_->fd(), 16384);
-    const int rc = std::get<0>(result);
-    const int error = std::get<1>(result);
-    ENVOY_CONN_LOG(trace, "read returns: {}", callbacks_->connection(), rc);
+    Api::SysCallResult result = buffer.read(callbacks_->fd(), 16384);
+    ENVOY_CONN_LOG(trace, "read returns: {}", callbacks_->connection(), result.rc_);
 
-    if (rc == 0) {
+    if (result.rc_ == 0) {
       // Remote close.
       end_stream = true;
       break;
-    } else if (rc == -1) {
+    } else if (result.rc_ == -1) {
       // Remote error (might be no data).
-      ENVOY_CONN_LOG(trace, "read error: {}", callbacks_->connection(), error);
-      if (error != EAGAIN) {
+      ENVOY_CONN_LOG(trace, "read error: {}", callbacks_->connection(), result.errno_);
+      if (result.errno_ != EAGAIN) {
         action = PostIoAction::Close;
       }
-
       break;
     } else {
-      bytes_read += rc;
+      bytes_read += result.rc_;
       if (callbacks_->shouldDrainReadBuffer()) {
         callbacks_->setReadBufferReady();
         break;
@@ -61,22 +58,20 @@ IoResult RawBufferSocket::doWrite(Buffer::Instance& buffer, bool end_stream) {
       action = PostIoAction::KeepOpen;
       break;
     }
-    std::tuple<int, int> result = buffer.write(callbacks_->fd());
-    const int rc = std::get<0>(result);
-    const int error = std::get<1>(result);
-    ENVOY_CONN_LOG(trace, "write returns: {}", callbacks_->connection(), rc);
-    if (rc == -1) {
-      ENVOY_CONN_LOG(trace, "write error: {} ({})", callbacks_->connection(), error,
-                     strerror(error));
-      if (error == EAGAIN) {
+    Api::SysCallResult result = buffer.write(callbacks_->fd());
+    ENVOY_CONN_LOG(trace, "write returns: {}", callbacks_->connection(), result.rc_);
+
+    if (result.rc_ == -1) {
+      ENVOY_CONN_LOG(trace, "write error: {} ({})", callbacks_->connection(), result.errno_,
+                     strerror(result.errno_));
+      if (result.errno_ == EAGAIN) {
         action = PostIoAction::KeepOpen;
       } else {
         action = PostIoAction::Close;
       }
-
       break;
     } else {
-      bytes_written += rc;
+      bytes_written += result.rc_;
     }
   } while (true);
 

test/common/buffer/owned_impl_test.cc

@@ -83,34 +83,34 @@ TEST_F(OwnedImplTest, Write) {
   Buffer::OwnedImpl buffer;
   buffer.add("example");
   EXPECT_CALL(os_sys_calls, writev(_, _, _)).WillOnce(Return(7));
-  std::tuple<int, int> result = buffer.write(-1);
-  EXPECT_EQ(7, std::get<0>(result));
+  Api::SysCallResult result = buffer.write(-1);
+  EXPECT_EQ(7, result.rc_);
   EXPECT_EQ(0, buffer.length());
 
   buffer.add("example");
   EXPECT_CALL(os_sys_calls, writev(_, _, _)).WillOnce(Return(6));
   result = buffer.write(-1);
-  EXPECT_EQ(6, std::get<0>(result));
+  EXPECT_EQ(6, result.rc_);
   EXPECT_EQ(1, buffer.length());
 
   EXPECT_CALL(os_sys_calls, writev(_, _, _)).WillOnce(Return(0));
   result = buffer.write(-1);
-  EXPECT_EQ(0, std::get<0>(result));
+  EXPECT_EQ(0, result.rc_);
   EXPECT_EQ(1, buffer.length());
 
   EXPECT_CALL(os_sys_calls, writev(_, _, _)).WillOnce(Return(-1));
   result = buffer.write(-1);
-  EXPECT_EQ(-1, std::get<0>(result));
+  EXPECT_EQ(-1, result.rc_);
   EXPECT_EQ(1, buffer.length());
 
   EXPECT_CALL(os_sys_calls, writev(_, _, _)).WillOnce(Return(1));
   result = buffer.write(-1);
-  EXPECT_EQ(1, std::get<0>(result));
+  EXPECT_EQ(1, result.rc_);
   EXPECT_EQ(0, buffer.length());
 
   EXPECT_CALL(os_sys_calls, writev(_, _, _)).Times(0);
   result = buffer.write(-1);
-  EXPECT_EQ(0, std::get<0>(result));
+  EXPECT_EQ(0, result.rc_);
   EXPECT_EQ(0, buffer.length());
 }
 
@@ -120,18 +120,18 @@ TEST_F(OwnedImplTest, Read) {
 
   Buffer::OwnedImpl buffer;
   EXPECT_CALL(os_sys_calls, readv(_, _, _)).WillOnce(Return(0));
-  std::tuple<int, int> result = buffer.read(-1, 100);
-  EXPECT_EQ(0, std::get<0>(result));
+  Api::SysCallResult result = buffer.read(-1, 100);
+  EXPECT_EQ(0, result.rc_);
   EXPECT_EQ(0, buffer.length());
 
   EXPECT_CALL(os_sys_calls, readv(_, _, _)).WillOnce(Return(-1));
   result = buffer.read(-1, 100);
-  EXPECT_EQ(-1, std::get<0>(result));
+  EXPECT_EQ(-1, result.rc_);
   EXPECT_EQ(0, buffer.length());
 
   EXPECT_CALL(os_sys_calls, readv(_, _, _)).Times(0);
   result = buffer.read(-1, 0);
-  EXPECT_EQ(0, std::get<0>(result));
+  EXPECT_EQ(0, result.rc_);
   EXPECT_EQ(0, buffer.length());
 }
 

test/common/buffer/watermark_buffer_test.cc

@@ -131,12 +131,11 @@ TEST_F(WatermarkBufferTest, WatermarkFdFunctions) {
 
   int bytes_written_total = 0;
   while (bytes_written_total < 20) {
-    std::tuple<int, int> result = buffer_.write(pipe_fds[1]);
-    int bytes_written = std::get<0>(result);
-    if (bytes_written < 0) {
-      ASSERT_EQ(EAGAIN, errno);
+    Api::SysCallResult result = buffer_.write(pipe_fds[1]);
+    if (result.rc_ < 0) {
+      ASSERT_EQ(EAGAIN, result.errno_);
     } else {
-      bytes_written_total += bytes_written;
+      bytes_written_total += result.rc_;
     }
   }
   EXPECT_EQ(1, times_high_watermark_called_);
@@ -145,8 +144,8 @@ TEST_F(WatermarkBufferTest, WatermarkFdFunctions) {
 
   int bytes_read_total = 0;
   while (bytes_read_total < 20) {
-    std::tuple<int, int> result = buffer_.read(pipe_fds[0], 20);
-    bytes_read_total += std::get<0>(result);
+    Api::SysCallResult result = buffer_.read(pipe_fds[0], 20);
+    bytes_read_total += result.rc_;
   }
   EXPECT_EQ(2, times_high_watermark_called_);
   EXPECT_EQ(20, buffer_.length());

test/common/network/connection_impl_test.cc

@@ -678,7 +678,7 @@ TEST_P(ConnectionImplTest, WriteWithWatermarks) {
   EXPECT_CALL(*client_write_buffer_, move(_))
       .WillRepeatedly(DoAll(AddBufferToStringWithoutDraining(&data_written),
                             Invoke(client_write_buffer_, &MockWatermarkBuffer::baseMove)));
-  EXPECT_CALL(*client_write_buffer_, write(_)).WillOnce(Invoke([&](int fd) -> std::tuple<int, int> {
+  EXPECT_CALL(*client_write_buffer_, write(_)).WillOnce(Invoke([&](int fd) -> Api::SysCallResult {
     dispatcher_->exit();
     return client_write_buffer_->failWrite(fd);
   }));
@@ -764,7 +764,7 @@ TEST_P(ConnectionImplTest, WatermarkFuzzing) {
         .WillOnce(Invoke(client_write_buffer_, &MockWatermarkBuffer::baseMove));
     EXPECT_CALL(*client_write_buffer_, write(_))
         .WillOnce(DoAll(Invoke([&](int) -> void { client_write_buffer_->drain(bytes_to_flush); }),
-                        Return(std::make_tuple(bytes_to_flush, 0))))
+                        Return(Api::SysCallResult{bytes_to_flush, 0})))
         .WillRepeatedly(testing::Invoke(client_write_buffer_, &MockWatermarkBuffer::failWrite));
     client_connection_->write(buffer_to_write, false);
     dispatcher_->run(Event::Dispatcher::RunType::NonBlock);

test/mocks/buffer/mocks.h

@@ -16,19 +16,18 @@ template <class BaseClass> class MockBufferBase : public BaseClass {
   MockBufferBase();
   MockBufferBase(std::function<void()> below_low, std::function<void()> above_high);
 
-  MOCK_METHOD1(write, std::tuple<int, int>(int fd));
+  MOCK_METHOD1(write, Api::SysCallResult(int fd));
   MOCK_METHOD1(move, void(Buffer::Instance& rhs));
   MOCK_METHOD2(move, void(Buffer::Instance& rhs, uint64_t length));
   MOCK_METHOD1(drain, void(uint64_t size));
 
   void baseMove(Buffer::Instance& rhs) { BaseClass::move(rhs); }
   void baseDrain(uint64_t size) { BaseClass::drain(size); }
 
-  std::tuple<int, int> trackWrites(int fd) {
-    std::tuple<int, int> result = BaseClass::write(fd);
-    int bytes_written = std::get<0>(result);
-    if (bytes_written > 0) {
-      bytes_written_ += bytes_written;
+  Api::SysCallResult trackWrites(int fd) {
+    Api::SysCallResult result = BaseClass::write(fd);
+    if (result.rc_ > 0) {
+      bytes_written_ += result.rc_;
     }
     return result;
   }
@@ -39,7 +38,7 @@ template <class BaseClass> class MockBufferBase : public BaseClass {
   }
 
   // A convenience function to invoke on write() which fails the write with EAGAIN.
-  std::tuple<int, int> failWrite(int) { return std::make_tuple(-1, EAGAIN); }
+  Api::SysCallResult failWrite(int) { return {-1, EAGAIN}; }
 
   int bytes_written() const { return bytes_written_; }
   uint64_t bytes_drained() const { return bytes_drained_; }