io_error: cleanup TODO

envoy/api/io_error.h

@@ -50,20 +50,21 @@ using IoErrorPtr = std::unique_ptr<IoError, IoErrorDeleterType>;
 /**
  * Basic type for return result which has a return code and error code defined
  * according to different implementations.
- * If the call succeeds, ok() should return true and |rc_| is valid. Otherwise |err_|
+ * If the call succeeds, ok() should return true and |return_value_| is valid. Otherwise |err_|
  * can be passed into IoError::getErrorCode() to extract the error. In this
- * case, |rc_| is invalid.
+ * case, |return_value_| is invalid.
  */
 template <typename ReturnValue> struct IoCallResult {
-  IoCallResult(ReturnValue rc, IoErrorPtr err) : rc_(rc), err_(std::move(err)) {}
+  IoCallResult(ReturnValue return_value, IoErrorPtr err)
+      : return_value_(return_value), err_(std::move(err)) {}
 
   IoCallResult(IoCallResult<ReturnValue>&& result) noexcept
-      : rc_(result.rc_), err_(std::move(result.err_)) {}
+      : return_value_(result.return_value_), err_(std::move(result.err_)) {}
 
   virtual ~IoCallResult() = default;
 
   IoCallResult& operator=(IoCallResult&& result) noexcept {
-    rc_ = result.rc_;
+    return_value_ = result.return_value_;
     err_ = std::move(result.err_);
     return *this;
   }
@@ -79,8 +80,7 @@ template <typename ReturnValue> struct IoCallResult {
    */
   bool wouldBlock() const { return !ok() && err_->getErrorCode() == IoError::IoErrorCode::Again; }
 
-  // TODO(danzh): rename it to be more meaningful, i.e. return_value_.
-  ReturnValue rc_;
+  ReturnValue return_value_;
   IoErrorPtr err_;
 };
 

envoy/api/os_sys_calls_common.h

@@ -15,7 +15,7 @@ template <typename T> struct SysCallResult {
   /**
    * The return code from the system call.
    */
-  T rc_;
+  T return_value_;
 
   /**
    * The errno value as captured after the system call.

source/common/access_log/access_log_manager_impl.cc

@@ -53,7 +53,7 @@ AccessLogFileImpl::AccessLogFileImpl(Filesystem::FilePtr&& file, Event::Dispatch
       thread_factory_(thread_factory), flush_interval_msec_(flush_interval_msec), stats_(stats) {
   flush_timer_->enableTimer(flush_interval_msec_);
   auto open_result = open();
-  if (!open_result.rc_) {
+  if (!open_result.return_value_) {
     throw EnvoyException(fmt::format("unable to open file '{}': {}", file_->path(),
                                      open_result.err_->getErrorDetails()));
   }
@@ -91,8 +91,8 @@ AccessLogFileImpl::~AccessLogFileImpl() {
       doWrite(flush_buffer_);
     }
     const Api::IoCallBoolResult result = file_->close();
-    ASSERT(result.rc_, fmt::format("unable to close file '{}': {}", file_->path(),
-                                   result.err_->getErrorDetails()));
+    ASSERT(result.return_value_, fmt::format("unable to close file '{}': {}", file_->path(),
+                                             result.err_->getErrorDetails()));
   }
 }
 
@@ -112,7 +112,7 @@ void AccessLogFileImpl::doWrite(Buffer::Instance& buffer) {
     for (const Buffer::RawSlice& slice : slices) {
       absl::string_view data(static_cast<char*>(slice.mem_), slice.len_);
       const Api::IoCallSizeResult result = file_->write(data);
-      if (result.ok() && result.rc_ == static_cast<ssize_t>(slice.len_)) {
+      if (result.ok() && result.return_value_ == static_cast<ssize_t>(slice.len_)) {
         stats_.write_completed_.inc();
       } else {
         // Probably disk full.
@@ -154,10 +154,10 @@ void AccessLogFileImpl::flushThreadFunc() {
       if (reopen_file_) {
         reopen_file_ = false;
         const Api::IoCallBoolResult result = file_->close();
-        ASSERT(result.rc_, fmt::format("unable to close file '{}': {}", file_->path(),
-                                       result.err_->getErrorDetails()));
+        ASSERT(result.return_value_, fmt::format("unable to close file '{}': {}", file_->path(),
+                                                 result.err_->getErrorDetails()));
         const Api::IoCallBoolResult open_result = open();
-        if (!open_result.rc_) {
+        if (!open_result.return_value_) {
           stats_.reopen_failed_.inc();
           return;
         }

source/common/api/win32/os_sys_calls_impl.cc

@@ -279,11 +279,11 @@ SysCallIntResult OsSysCallsImpl::socketpair(int domain, int type, int protocol,
   sv[0] = sv[1] = INVALID_SOCKET;
 
   SysCallSocketResult socket_result = socket(domain, type, protocol);
-  if (SOCKET_INVALID(socket_result.rc_)) {
+  if (SOCKET_INVALID(socket_result.return_value_)) {
     return {SOCKET_ERROR, socket_result.errno_};
   }
 
-  os_fd_t listener = socket_result.rc_;
+  os_fd_t listener = socket_result.return_value_;
 
   typedef union {
     struct sockaddr_storage sa;
@@ -313,44 +313,44 @@ SysCallIntResult OsSysCallsImpl::socketpair(int domain, int type, int protocol,
   };
 
   SysCallIntResult int_result = bind(listener, reinterpret_cast<sockaddr*>(&a), sa_size);
-  if (int_result.rc_ == SOCKET_ERROR) {
+  if (int_result.return_value_ == SOCKET_ERROR) {
     onErr();
     return int_result;
   }
 
   int_result = listen(listener, 1);
-  if (int_result.rc_ == SOCKET_ERROR) {
+  if (int_result.return_value_ == SOCKET_ERROR) {
     onErr();
     return int_result;
   }
 
   socket_result = socket(domain, type, protocol);
-  if (SOCKET_INVALID(socket_result.rc_)) {
+  if (SOCKET_INVALID(socket_result.return_value_)) {
     onErr();
     return {SOCKET_ERROR, socket_result.errno_};
   }
-  sv[0] = socket_result.rc_;
+  sv[0] = socket_result.return_value_;
 
   a = {};
   int_result = getsockname(listener, reinterpret_cast<sockaddr*>(&a), &sa_size);
-  if (int_result.rc_ == SOCKET_ERROR) {
+  if (int_result.return_value_ == SOCKET_ERROR) {
     onErr();
     return int_result;
   }
 
   int_result = connect(sv[0], reinterpret_cast<sockaddr*>(&a), sa_size);
-  if (int_result.rc_ == SOCKET_ERROR) {
+  if (int_result.return_value_ == SOCKET_ERROR) {
     onErr();
     return int_result;
   }
 
-  socket_result.rc_ = ::accept(listener, nullptr, nullptr);
-  if (SOCKET_INVALID(socket_result.rc_)) {
+  socket_result.return_value_ = ::accept(listener, nullptr, nullptr);
+  if (SOCKET_INVALID(socket_result.return_value_)) {
     socket_result.errno_ = ::WSAGetLastError();
     onErr();
     return {SOCKET_ERROR, socket_result.errno_};
   }
-  sv[1] = socket_result.rc_;
+  sv[1] = socket_result.return_value_;
 
   ::closesocket(listener);
   return {0, 0};

source/common/event/win32/signal_impl.cc

@@ -21,14 +21,14 @@ SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num
   os_fd_t socks[2];
   Api::SysCallIntResult result =
       Api::OsSysCallsSingleton::get().socketpair(AF_INET, SOCK_STREAM, IPPROTO_TCP, socks);
-  ASSERT(result.rc_ == 0);
+  ASSERT(result.return_value_ == 0);
 
   read_handle_ = std::make_unique<Network::IoSocketHandleImpl>(socks[0], false, AF_INET);
   result = read_handle_->setBlocking(false);
-  ASSERT(result.rc_ == 0);
+  ASSERT(result.return_value_ == 0);
   auto write_handle = std::make_shared<Network::IoSocketHandleImpl>(socks[1], false, AF_INET);
   result = write_handle->setBlocking(false);
-  ASSERT(result.rc_ == 0);
+  ASSERT(result.return_value_ == 0);
 
   read_handle_->initializeFileEvent(
       dispatcher,

source/common/filesystem/posix/directory_iterator_impl.cc

@@ -55,7 +55,7 @@ FileType DirectoryIteratorImpl::fileType(const std::string& full_path,
   struct stat stat_buf;
 
   const Api::SysCallIntResult result = os_sys_calls.stat(full_path.c_str(), &stat_buf);
-  if (result.rc_ != 0) {
+  if (result.return_value_ != 0) {
     if (errno == ENOENT) {
       // Special case. This directory entity is likely to be a symlink,
       // but the reference is broken as the target could not be stat()'ed.

source/common/filesystem/posix/filesystem_impl.cc

@@ -25,8 +25,9 @@ namespace Filesystem {
 
 FileImplPosix::~FileImplPosix() {
   if (isOpen()) {
+    // NOLINTNEXTLINE(clang-analyzer-optin.cplusplus.VirtualCall)
     const Api::IoCallBoolResult result = close();
-    ASSERT(result.rc_);
+    ASSERT(result.return_value_);
   }
 }
 
@@ -152,7 +153,7 @@ bool InstanceImplPosix::illegalPath(const std::string& path) {
   }
 
   const Api::SysCallStringResult canonical_path = canonicalPath(path);
-  if (canonical_path.rc_.empty()) {
+  if (canonical_path.return_value_.empty()) {
     ENVOY_LOG_MISC(debug, "Unable to determine canonical path for {}: {}", path,
                    errorDetails(canonical_path.errno_));
     return true;
@@ -163,9 +164,9 @@ bool InstanceImplPosix::illegalPath(const std::string& path) {
   // platform in the future, growing these or relaxing some constraints (e.g.
   // there are valid reasons to go via /proc for file paths).
   // TODO(htuch): Optimize this as a hash lookup if we grow any further.
-  if (absl::StartsWith(canonical_path.rc_, "/dev") ||
-      absl::StartsWith(canonical_path.rc_, "/sys") ||
-      absl::StartsWith(canonical_path.rc_, "/proc")) {
+  if (absl::StartsWith(canonical_path.return_value_, "/dev") ||
+      absl::StartsWith(canonical_path.return_value_, "/sys") ||
+      absl::StartsWith(canonical_path.return_value_, "/proc")) {
     return true;
   }
   return false;

source/common/filesystem/win32/filesystem_impl.cc

@@ -22,7 +22,7 @@ namespace Filesystem {
 FileImplWin32::~FileImplWin32() {
   if (isOpen()) {
     const Api::IoCallBoolResult result = close();
-    ASSERT(result.rc_);
+    ASSERT(result.return_value_);
   }
 }
 

source/common/filesystem/win32/watcher_impl.cc

@@ -11,14 +11,14 @@ WatcherImpl::WatcherImpl(Event::Dispatcher& dispatcher, Api::Api& api)
     : api_(api), os_sys_calls_(Api::OsSysCallsSingleton::get()) {
   os_fd_t socks[2];
   Api::SysCallIntResult result = os_sys_calls_.socketpair(AF_INET, SOCK_STREAM, IPPROTO_TCP, socks);
-  ASSERT(result.rc_ == 0);
+  ASSERT(result.return_value_ == 0);
 
   read_handle_ = std::make_unique<Network::IoSocketHandleImpl>(socks[0], false, AF_INET);
   result = read_handle_->setBlocking(false);
-  ASSERT(result.rc_ == 0);
+  ASSERT(result.return_value_ == 0);
   write_handle_ = std::make_unique<Network::IoSocketHandleImpl>(socks[1], false, AF_INET);
   result = write_handle_->setBlocking(false);
-  ASSERT(result.rc_ == 0);
+  ASSERT(result.return_value_ == 0);
 
   read_handle_->initializeFileEvent(
       dispatcher,
@@ -154,7 +154,7 @@ void WatcherImpl::endDirectoryWatch(Network::IoHandle& io_handle, HANDLE event_h
   constexpr absl::string_view data{"a"};
   buffer.add(data);
   auto result = io_handle.write(buffer);
-  RELEASE_ASSERT(result.rc_ == 1,
+  RELEASE_ASSERT(result.return_value_ == 1,
                  fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails()));
 }
 
@@ -207,7 +207,7 @@ void WatcherImpl::directoryChangeCompletion(DWORD err, DWORD num_bytes, LPOVERLA
         // not in this completion routine
         Buffer::RawSlice buffer{(void*)data.data(), 1};
         auto result = watcher->write_handle_->writev(&buffer, 1);
-        RELEASE_ASSERT(result.rc_ == 1,
+        RELEASE_ASSERT(result.return_value_ == 1,
                        fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails()));
       }
     }

source/common/formatter/substitution_formatter.cc

@@ -93,7 +93,7 @@ const absl::optional<std::string> SubstitutionFormatUtils::getHostname() {
   const Api::SysCallIntResult result = os_sys_calls.gethostname(name, len);
 
   absl::optional<std::string> hostname;
-  if (result.rc_ == 0) {
+  if (result.return_value_ == 0) {
     hostname = name;
   }
 

source/common/network/connection_impl.cc

@@ -288,22 +288,23 @@ void ConnectionImpl::noDelay(bool enable) {
   Api::SysCallIntResult result =
       socket_->setSocketOption(IPPROTO_TCP, TCP_NODELAY, &new_value, sizeof(new_value));
 #if defined(__APPLE__)
-  if (SOCKET_FAILURE(result.rc_) && result.errno_ == SOCKET_ERROR_INVAL) {
+  if (SOCKET_FAILURE(result.return_value_) && result.errno_ == SOCKET_ERROR_INVAL) {
     // Sometimes occurs when the connection is not yet fully formed. Empirically, TCP_NODELAY is
     // enabled despite this result.
     return;
   }
 #elif defined(WIN32)
-  if (SOCKET_FAILURE(result.rc_) &&
+  if (SOCKET_FAILURE(result.return_value_) &&
       (result.errno_ == SOCKET_ERROR_AGAIN || result.errno_ == SOCKET_ERROR_INVAL)) {
     // Sometimes occurs when the connection is not yet fully formed. Empirically, TCP_NODELAY is
     // enabled despite this result.
     return;
   }
 #endif
 
-  RELEASE_ASSERT(result.rc_ == 0, fmt::format("Failed to set TCP_NODELAY with error {}, {}",
-                                              result.errno_, errorDetails(result.errno_)));
+  RELEASE_ASSERT(result.return_value_ == 0,
+                 fmt::format("Failed to set TCP_NODELAY with error {}, {}", result.errno_,
+                             errorDetails(result.errno_)));
 }
 
 void ConnectionImpl::onRead(uint64_t read_buffer_size) {
@@ -648,7 +649,7 @@ ConnectionImpl::unixSocketPeerCredentials() const {
 #else
   struct ucred ucred;
   socklen_t ucred_size = sizeof(ucred);
-  int rc = socket_->getSocketOption(SOL_SOCKET, SO_PEERCRED, &ucred, &ucred_size).rc_;
+  int rc = socket_->getSocketOption(SOL_SOCKET, SO_PEERCRED, &ucred, &ucred_size).return_value_;
   if (SOCKET_FAILURE(rc)) {
     return absl::nullopt;
   }
@@ -663,8 +664,8 @@ void ConnectionImpl::onWriteReady() {
   if (connecting_) {
     int error;
     socklen_t error_size = sizeof(error);
-    RELEASE_ASSERT(socket_->getSocketOption(SOL_SOCKET, SO_ERROR, &error, &error_size).rc_ == 0,
-                   "");
+    RELEASE_ASSERT(
+        socket_->getSocketOption(SOL_SOCKET, SO_ERROR, &error, &error_size).return_value_ == 0, "");
 
     if (error == 0) {
       ENVOY_CONN_LOG(debug, "connected", *this);
@@ -846,7 +847,7 @@ ClientConnectionImpl::ClientConnectionImpl(
 
   if (*source != nullptr) {
     Api::SysCallIntResult result = socket_->bind(*source);
-    if (result.rc_ < 0) {
+    if (result.return_value_ < 0) {
       // TODO(lizan): consider add this error into transportFailureReason.
       ENVOY_LOG_MISC(debug, "Bind failure. Failed to bind to {}: {}", source->get()->asString(),
                      errorDetails(result.errno_));
@@ -865,13 +866,13 @@ void ClientConnectionImpl::connect() {
   ENVOY_CONN_LOG(debug, "connecting to {}", *this,
                  socket_->addressProvider().remoteAddress()->asString());
   const Api::SysCallIntResult result = socket_->connect(socket_->addressProvider().remoteAddress());
-  if (result.rc_ == 0) {
+  if (result.return_value_ == 0) {
     // write will become ready.
     ASSERT(connecting_);
     return;
   }
 
-  ASSERT(SOCKET_FAILURE(result.rc_));
+  ASSERT(SOCKET_FAILURE(result.return_value_));
 #ifdef WIN32
   // winsock2 connect returns EWOULDBLOCK if the socket is non-blocking and the connection
   // cannot be completed immediately. We do not check for `EINPROGRESS` as that error is for