From cdec4fa61cf08c4420ad3a1ccb209771e9afeb8a Mon Sep 17 00:00:00 2001
From: Benjamin Peterson <benjamin@engflow.com>
Date: Tue, 26 Dec 2023 10:40:56 -0800
Subject: [PATCH] [EventEngine] Improve PosixEventEngine::ConnectInternal error
 handling (#35356)

Rename `saved_errno` to `connect_errno`.

Avoid relying on `errno` being zero if `connect(2)` does not fail.

Slightly linearize control flow.

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes #35356

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35356 from benjaminp:connect-errno 0dabdf0562ac1d70667688d2cd3638794299ec86
PiperOrigin-RevId: 593819124
---
 .../event_engine/posix_engine/posix_engine.cc | 22 +++++++++----------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/core/lib/event_engine/posix_engine/posix_engine.cc b/src/core/lib/event_engine/posix_engine/posix_engine.cc
index a5a4ee1b40f..06938a6204a 100644
--- a/src/core/lib/event_engine/posix_engine/posix_engine.cc
+++ b/src/core/lib/event_engine/posix_engine/posix_engine.cc
@@ -255,11 +255,11 @@ EventEngine::ConnectionHandle PosixEventEngine::ConnectInternal(
     MemoryAllocator&& allocator, const PosixTcpOptions& options,
     Duration timeout) {
   int err;
-  int saved_errno;
+  int connect_errno;
   do {
     err = connect(sock.Fd(), addr.address(), addr.size());
   } while (err < 0 && errno == EINTR);
-  saved_errno = errno;
+  connect_errno = (err < 0) ? errno : 0;
 
   auto addr_uri = ResolvedAddressToURI(addr);
   if (!addr_uri.ok()) {
@@ -274,13 +274,8 @@ EventEngine::ConnectionHandle PosixEventEngine::ConnectInternal(
   PosixEventPoller* poller = poller_manager_->Poller();
   EventHandle* handle =
       poller->CreateHandle(sock.Fd(), name, poller->CanTrackErrors());
-  int64_t connection_id = 0;
-  if (saved_errno == EWOULDBLOCK || saved_errno == EINPROGRESS) {
-    // Connection is still in progress.
-    connection_id = last_connection_id_.fetch_add(1, std::memory_order_acq_rel);
-  }
 
-  if (err >= 0) {
+  if (connect_errno == 0) {
     // Connection already succeded. Return 0 to discourage any cancellation
     // attempts.
     Run([on_connect = std::move(on_connect),
@@ -290,18 +285,21 @@ EventEngine::ConnectionHandle PosixEventEngine::ConnectInternal(
     });
     return EventEngine::ConnectionHandle::kInvalid;
   }
-  if (saved_errno != EWOULDBLOCK && saved_errno != EINPROGRESS) {
+  if (connect_errno != EWOULDBLOCK && connect_errno != EINPROGRESS) {
     // Connection already failed. Return 0 to discourage any cancellation
     // attempts.
     handle->OrphanHandle(nullptr, nullptr, "tcp_client_connect_error");
     Run([on_connect = std::move(on_connect),
-         ep = absl::FailedPreconditionError(
-             absl::StrCat("connect failed: ", "addr: ", addr_uri.value(),
-                          " error: ", std::strerror(saved_errno)))]() mutable {
+         ep = absl::FailedPreconditionError(absl::StrCat(
+             "connect failed: ", "addr: ", addr_uri.value(),
+             " error: ", std::strerror(connect_errno)))]() mutable {
       on_connect(std::move(ep));
     });
     return EventEngine::ConnectionHandle::kInvalid;
   }
+  // Connection is still in progress.
+  int64_t connection_id =
+      last_connection_id_.fetch_add(1, std::memory_order_acq_rel);
   AsyncConnect* ac = new AsyncConnect(
       std::move(on_connect), shared_from_this(), executor_.get(), handle,
       std::move(allocator), options, addr_uri.value(), connection_id);