From 516d1769fca762eee76be06e45e03886bd00b0ef Mon Sep 17 00:00:00 2001
From: Kevin Schoedel <kpschoedel@google.com>
Date: Fri, 22 Oct 2021 12:03:40 -0400
Subject: [PATCH 1/2] Various small revisions to TCPEndPoint

#### Problem

Working toward #7715 Virtualize System and Inet interfaces

#### Change overview

Miscellaneous revisions with no change to functionality.

- Add `TCPTest` class for test access to internal `TCPEndPoint` state.
- Make `State` private.
- Favor `ReturnErrorOnFailure()` etc.
- Factor `GetSocketInfo()` out of `Get(Local|Peer)Info()`

#### Testing

CI; no changes to functionality intended.
---
 src/inet/TCPEndPoint.cpp         | 319 +++++++++++++------------------
 src/inet/TCPEndPoint.h           |  72 ++++---
 src/inet/tests/TestInetLayer.cpp |  45 ++---
 3 files changed, 194 insertions(+), 242 deletions(-)

diff --git a/src/inet/TCPEndPoint.cpp b/src/inet/TCPEndPoint.cpp
index 4c807ec7016e9c..f9ecba7671545a 100644
--- a/src/inet/TCPEndPoint.cpp
+++ b/src/inet/TCPEndPoint.cpp
@@ -63,10 +63,8 @@
 #include <unistd.h>
 
 // SOCK_CLOEXEC not defined on all platforms, e.g. iOS/macOS:
-#ifdef SOCK_CLOEXEC
-#define SOCK_FLAGS SOCK_CLOEXEC
-#else
-#define SOCK_FLAGS 0
+#ifndef SOCK_CLOEXEC
+#define SOCK_CLOEXEC 0
 #endif
 
 #if defined(SOL_TCP)
@@ -265,15 +263,13 @@ CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, Inter
 
 CHIP_ERROR TCPEndPoint::GetPeerInfo(IPAddress * retAddr, uint16_t * retPort) const
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
     // Lock LwIP stack
     LOCK_TCPIP_CORE();
 
-    if (mTCP != NULL)
+    CHIP_ERROR res = CHIP_ERROR_CONNECTION_ABORTED;
+    if (mTCP != nullptr)
     {
         *retPort = mTCP->remote_port;
 
@@ -286,9 +282,8 @@ CHIP_ERROR TCPEndPoint::GetPeerInfo(IPAddress * retAddr, uint16_t * retPort) con
         *retAddr                    = IPAddress(mTCP->remote_ip.ip6);
 #endif // !INET_CONFIG_ENABLE_IPV4
 #endif // LWIP_VERSION_MAJOR <= 1 || LWIP_VERSION_MINOR >= 5
+        res = CHIP_NO_ERROR;
     }
-    else
-        res = CHIP_ERROR_CONNECTION_ABORTED;
 
     // Unlock LwIP stack
     UNLOCK_TCPIP_CORE();
@@ -296,17 +291,15 @@ CHIP_ERROR TCPEndPoint::GetPeerInfo(IPAddress * retAddr, uint16_t * retPort) con
     return res;
 }
 
-CHIP_ERROR TCPEndPoint::GetLocalInfo(IPAddress * retAddr, uint16_t * retPort)
+CHIP_ERROR TCPEndPoint::GetLocalInfo(IPAddress * retAddr, uint16_t * retPort) const
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
     // Lock LwIP stack
     LOCK_TCPIP_CORE();
 
-    if (mTCP != NULL)
+    CHIP_ERROR res = CHIP_ERROR_CONNECTION_ABORTED;
+    if (mTCP != nullptr)
     {
         *retPort = mTCP->local_port;
 
@@ -319,9 +312,8 @@ CHIP_ERROR TCPEndPoint::GetLocalInfo(IPAddress * retAddr, uint16_t * retPort)
         *retAddr                    = IPAddress(mTCP->local_ip.ip6);
 #endif // !INET_CONFIG_ENABLE_IPV4
 #endif // LWIP_VERSION_MAJOR <= 1 || LWIP_VERSION_MINOR >= 5
+        res = CHIP_NO_ERROR;
     }
-    else
-        res = CHIP_ERROR_CONNECTION_ABORTED;
 
     // Unlock LwIP stack
     UNLOCK_TCPIP_CORE();
@@ -331,8 +323,8 @@ CHIP_ERROR TCPEndPoint::GetLocalInfo(IPAddress * retAddr, uint16_t * retPort)
 
 CHIP_ERROR TCPEndPoint::GetInterfaceId(InterfaceId * retInterface)
 {
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
+
     // TODO: Does netif_get_by_index(mTCP->netif_idx) do the right thing?  I
     // can't quite tell whether LwIP supports a specific interface id for TCP at
     // all.  For now just claim no particular interface id.
@@ -355,18 +347,17 @@ CHIP_ERROR TCPEndPoint::SendQueuedImpl(bool queueWasEmpty)
 
 CHIP_ERROR TCPEndPoint::EnableNoDelay()
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
     // Lock LwIP stack
     LOCK_TCPIP_CORE();
 
-    if (mTCP != NULL)
+    CHIP_ERROR res = CHIP_ERROR_CONNECTION_ABORTED;
+    if (mTCP != nullptr)
+    {
         tcp_nagle_disable(mTCP);
-    else
-        res = CHIP_ERROR_CONNECTION_ABORTED;
+        res = CHIP_NO_ERROR;
+    }
 
     // Unlock LwIP stack
     UNLOCK_TCPIP_CORE();
@@ -376,10 +367,8 @@ CHIP_ERROR TCPEndPoint::EnableNoDelay()
 
 CHIP_ERROR TCPEndPoint::EnableKeepAlive(uint16_t interval, uint16_t timeoutCount)
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
+    CHIP_ERROR res = CHIP_ERROR_NOT_IMPLEMENTED;
 
 #if LWIP_TCP_KEEPALIVE
 
@@ -399,17 +388,16 @@ CHIP_ERROR TCPEndPoint::EnableKeepAlive(uint16_t interval, uint16_t timeoutCount
 
         // Enable keepalives for the connection.
         ip_set_option(mTCP, SOF_KEEPALIVE);
+        res = CHIP_NO_ERROR;
     }
     else
+    {
         res = CHIP_ERROR_CONNECTION_ABORTED;
+    }
 
     // Unlock LwIP stack
     UNLOCK_TCPIP_CORE();
 
-#else // LWIP_TCP_KEEPALIVE
-
-    res = CHIP_ERROR_NOT_IMPLEMENTED;
-
 #endif // LWIP_TCP_KEEPALIVE
 
     return res;
@@ -417,10 +405,8 @@ CHIP_ERROR TCPEndPoint::EnableKeepAlive(uint16_t interval, uint16_t timeoutCount
 
 CHIP_ERROR TCPEndPoint::DisableKeepAlive()
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
+    CHIP_ERROR res = CHIP_ERROR_NOT_IMPLEMENTED;
 
 #if LWIP_TCP_KEEPALIVE
 
@@ -431,38 +417,17 @@ CHIP_ERROR TCPEndPoint::DisableKeepAlive()
     {
         // Disable keepalives on the connection.
         ip_reset_option(mTCP, SOF_KEEPALIVE);
+        res = CHIP_NO_ERROR;
     }
     else
+    {
         res = CHIP_ERROR_CONNECTION_ABORTED;
+    }
 
     // Unlock LwIP stack
     UNLOCK_TCPIP_CORE();
 
-#else // LWIP_TCP_KEEPALIVE
-
-    res = CHIP_ERROR_NOT_IMPLEMENTED;
-
 #endif // LWIP_TCP_KEEPALIVE
-    return res;
-}
-
-CHIP_ERROR TCPEndPoint::AckReceive(uint16_t len)
-{
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
-
-    // Lock LwIP stack
-    LOCK_TCPIP_CORE();
-
-    if (mTCP != NULL)
-        tcp_recved(mTCP, len);
-    else
-        res = CHIP_ERROR_CONNECTION_ABORTED;
-
-    // Unlock LwIP stack
-    UNLOCK_TCPIP_CORE();
 
     return res;
 }
@@ -657,6 +622,27 @@ void TCPEndPoint::DoCloseImpl(CHIP_ERROR err, State oldState)
     }
 }
 
+CHIP_ERROR TCPEndPoint::AckReceive(uint16_t len)
+{
+    CHIP_ERROR res = CHIP_NO_ERROR;
+
+    if (!IsConnected())
+        return CHIP_ERROR_INCORRECT_STATE;
+
+    // Lock LwIP stack
+    LOCK_TCPIP_CORE();
+
+    if (mTCP != nullptr)
+        tcp_recved(mTCP, len);
+    else
+        res = CHIP_ERROR_CONNECTION_ABORTED;
+
+    // Unlock LwIP stack
+    UNLOCK_TCPIP_CORE();
+
+    return res;
+}
+
 #if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
 void TCPEndPoint::TCPUserTimeoutHandler(chip::System::Layer * aSystemLayer, void * aAppState)
 {
@@ -1194,7 +1180,9 @@ CHIP_ERROR TCPEndPoint::BindImpl(IPAddressType addrType, const IPAddress & addr,
             sa.sin6_scope_id = 0;
 
             if (bind(mSocket, reinterpret_cast<const sockaddr *>(&sa), static_cast<unsigned>(sizeof(sa))) != 0)
+            {
                 res = CHIP_ERROR_POSIX(errno);
+            }
         }
 #if INET_CONFIG_ENABLE_IPV4
         else if (addrType == IPAddressType::kIPv4)
@@ -1206,11 +1194,15 @@ CHIP_ERROR TCPEndPoint::BindImpl(IPAddressType addrType, const IPAddress & addr,
             sa.sin_addr   = addr.ToIPv4();
 
             if (bind(mSocket, reinterpret_cast<const sockaddr *>(&sa), static_cast<unsigned>(sizeof(sa))) != 0)
+            {
                 res = CHIP_ERROR_POSIX(errno);
+            }
         }
 #endif // INET_CONFIG_ENABLE_IPV4
         else
+        {
             res = INET_ERROR_WRONG_ADDRESS_TYPE;
+        }
     }
 
 #if CHIP_SYSTEM_CONFIG_USE_DISPATCH
@@ -1242,35 +1234,31 @@ CHIP_ERROR TCPEndPoint::BindImpl(IPAddressType addrType, const IPAddress & addr,
 
 CHIP_ERROR TCPEndPoint::ListenImpl(uint16_t backlog)
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
     if (listen(mSocket, backlog) != 0)
     {
-        res = CHIP_ERROR_POSIX(errno);
+        return CHIP_ERROR_POSIX(errno);
     }
-    else
-    {
-        // Enable non-blocking mode for the socket.
-        int flags = fcntl(mSocket, F_GETFL, 0);
-        fcntl(mSocket, F_SETFL, flags | O_NONBLOCK);
 
-        // Wait for ability to read on this endpoint.
-        res = static_cast<System::LayerSockets *>(Layer().SystemLayer())
-                  ->SetCallback(mWatch, HandlePendingIO, reinterpret_cast<intptr_t>(this));
-        if (res == CHIP_NO_ERROR)
-        {
-            res = static_cast<System::LayerSockets *>(Layer().SystemLayer())->RequestCallbackOnPendingRead(mWatch);
-        }
+    // Enable non-blocking mode for the socket.
+    int flags = fcntl(mSocket, F_GETFL, 0);
+    fcntl(mSocket, F_SETFL, flags | O_NONBLOCK);
+
+    // Wait for ability to read on this endpoint.
+    CHIP_ERROR res = static_cast<System::LayerSockets *>(Layer().SystemLayer())
+                         ->SetCallback(mWatch, HandlePendingIO, reinterpret_cast<intptr_t>(this));
+    if (res == CHIP_NO_ERROR)
+    {
+        res = static_cast<System::LayerSockets *>(Layer().SystemLayer())->RequestCallbackOnPendingRead(mWatch);
     }
+
     return res;
 }
 
 CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, InterfaceId intfId)
 {
     IPAddressType addrType = addr.Type();
-    CHIP_ERROR res         = GetSocket(addrType);
-    if (res != CHIP_NO_ERROR)
-        return res;
+
+    ReturnErrorOnFailure(GetSocket(addrType));
 
     if (intfId == INET_NULL_INTERFACEID)
     {
@@ -1292,9 +1280,7 @@ CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, Inter
             struct ::ifreq ifr;
             memset(&ifr, 0, sizeof(ifr));
 
-            res = GetInterfaceName(intfId, ifr.ifr_name, sizeof(ifr.ifr_name));
-            if (res != CHIP_NO_ERROR)
-                return res;
+            ReturnErrorOnFailure(GetInterfaceName(intfId, ifr.ifr_name, sizeof(ifr.ifr_name)));
 
             // Attempt to bind to the interface using SO_BINDTODEVICE which requires privileged access.
             // If the permission is denied(EACCES) because CHIP is running in a context
@@ -1303,7 +1289,7 @@ CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, Inter
             int r = setsockopt(mSocket, SOL_SOCKET, SO_BINDTODEVICE, &ifr, sizeof(ifr));
             if (r < 0 && errno != EACCES)
             {
-                return res = CHIP_ERROR_POSIX(errno);
+                return CHIP_ERROR_POSIX(errno);
             }
 
             if (r < 0)
@@ -1312,9 +1298,7 @@ CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, Inter
                 // Attempting to initiate a connection via a specific interface is not allowed.
                 // The only way to do this is to bind the local to an address on the desired
                 // interface.
-                res = BindSrcAddrFromIntf(addrType, intfId);
-                if (res != CHIP_NO_ERROR)
-                    return res;
+                ReturnErrorOnFailure(BindSrcAddrFromIntf(addrType, intfId));
             }
         }
     }
@@ -1356,13 +1340,15 @@ CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, Inter
     }
 #endif // INET_CONFIG_ENABLE_IPV4
     else
+    {
         return INET_ERROR_WRONG_ADDRESS_TYPE;
+    }
 
     int conRes = connect(mSocket, sockaddrptr, sockaddrsize);
 
     if (conRes == -1 && errno != EINPROGRESS)
     {
-        res = CHIP_ERROR_POSIX(errno);
+        CHIP_ERROR res = CHIP_ERROR_POSIX(errno);
         DoClose(res, true);
         return res;
     }
@@ -1389,72 +1375,49 @@ CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, Inter
         ReturnErrorOnFailure(static_cast<System::LayerSockets *>(Layer().SystemLayer())->RequestCallbackOnPendingWrite(mWatch));
     }
 
-    return res;
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR TCPEndPoint::GetPeerInfo(IPAddress * retAddr, uint16_t * retPort) const
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
-
-    SockAddr sa;
-    memset(&sa, 0, sizeof(sa));
-    socklen_t saLen = sizeof(sa);
-
-    if (getpeername(mSocket, &sa.any, &saLen) != 0)
-        return CHIP_ERROR_POSIX(errno);
-
-    if (sa.any.sa_family == AF_INET6)
-    {
-        *retAddr = IPAddress(sa.in6.sin6_addr);
-        *retPort = ntohs(sa.in6.sin6_port);
-    }
-#if INET_CONFIG_ENABLE_IPV4
-    else if (sa.any.sa_family == AF_INET)
-    {
-        *retAddr = IPAddress(sa.in.sin_addr);
-        *retPort = ntohs(sa.in.sin_port);
-    }
-#endif // INET_CONFIG_ENABLE_IPV4
-    else
-        return CHIP_ERROR_INCORRECT_STATE;
-
-    return res;
+    return GetSocketInfo(getpeername, retAddr, retPort);
 }
 
-CHIP_ERROR TCPEndPoint::GetLocalInfo(IPAddress * retAddr, uint16_t * retPort)
+CHIP_ERROR TCPEndPoint::GetLocalInfo(IPAddress * retAddr, uint16_t * retPort) const
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
+    return GetSocketInfo(getsockname, retAddr, retPort);
+}
 
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+CHIP_ERROR TCPEndPoint::GetSocketInfo(int getname(int, sockaddr *, socklen_t *), IPAddress * retAddr, uint16_t * retPort) const
+{
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
     SockAddr sa;
-
     memset(&sa, 0, sizeof(sa));
     socklen_t saLen = sizeof(sa);
 
-    if (getsockname(mSocket, &sa.any, &saLen) != 0)
+    if (getname(mSocket, &sa.any, &saLen) != 0)
+    {
         return CHIP_ERROR_POSIX(errno);
+    }
 
     if (sa.any.sa_family == AF_INET6)
     {
         *retAddr = IPAddress(sa.in6.sin6_addr);
         *retPort = ntohs(sa.in6.sin6_port);
+        return CHIP_NO_ERROR;
     }
+
 #if INET_CONFIG_ENABLE_IPV4
-    else if (sa.any.sa_family == AF_INET)
+    if (sa.any.sa_family == AF_INET)
     {
         *retAddr = IPAddress(sa.in.sin_addr);
         *retPort = ntohs(sa.in.sin_port);
+        return CHIP_NO_ERROR;
     }
 #endif // INET_CONFIG_ENABLE_IPV4
-    else
-        return CHIP_ERROR_INCORRECT_STATE;
 
-    return res;
+    return CHIP_ERROR_INCORRECT_STATE;
 }
 
 CHIP_ERROR TCPEndPoint::GetInterfaceId(InterfaceId * retInterface)
@@ -1495,7 +1458,7 @@ CHIP_ERROR TCPEndPoint::GetInterfaceId(InterfaceId * retInterface)
 #endif // INET_CONFIG_ENABLE_IPV4
 
     *retInterface = INET_NULL_INTERFACEID;
-    return CHIP_ERROR_INCORRECT_STATE;
+    return INET_ERROR_WRONG_ADDRESS_TYPE;
 }
 
 CHIP_ERROR TCPEndPoint::SendQueuedImpl(bool queueWasEmpty)
@@ -1510,82 +1473,72 @@ CHIP_ERROR TCPEndPoint::SendQueuedImpl(bool queueWasEmpty)
 
 CHIP_ERROR TCPEndPoint::EnableNoDelay()
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
-
-    {
-        int val;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
 #ifdef TCP_NODELAY
-        // Disable TCP Nagle buffering by setting TCP_NODELAY socket option to true
-        val = 1;
-        if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_NODELAY, &val, sizeof(val)) != 0)
-            return CHIP_ERROR_POSIX(errno);
-#endif // defined(TCP_NODELAY)
+    // Disable TCP Nagle buffering by setting TCP_NODELAY socket option to true
+    int val = 1;
+    if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_NODELAY, &val, sizeof(val)) != 0)
+    {
+        return CHIP_ERROR_POSIX(errno);
     }
+#endif // defined(TCP_NODELAY)
 
-    return res;
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR TCPEndPoint::EnableKeepAlive(uint16_t interval, uint16_t timeoutCount)
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
+    // Set the idle interval
+    int val = interval;
+    if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_IDLE_INTERVAL_OPT_NAME, &val, sizeof(val)) != 0)
     {
-        int val;
-
-        // Set the idle interval
-        val = interval;
-        if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_IDLE_INTERVAL_OPT_NAME, &val, sizeof(val)) != 0)
-            return CHIP_ERROR_POSIX(errno);
+        return CHIP_ERROR_POSIX(errno);
+    }
 
-        // Set the probe retransmission interval.
-        val = interval;
-        if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_KEEPINTVL, &val, sizeof(val)) != 0)
-            return CHIP_ERROR_POSIX(errno);
+    // Set the probe retransmission interval.
+    val = interval;
+    if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_KEEPINTVL, &val, sizeof(val)) != 0)
+    {
+        return CHIP_ERROR_POSIX(errno);
+    }
 
-        // Set the probe timeout count
-        val = timeoutCount;
-        if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_KEEPCNT, &val, sizeof(val)) != 0)
-            return CHIP_ERROR_POSIX(errno);
+    // Set the probe timeout count
+    val = timeoutCount;
+    if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_KEEPCNT, &val, sizeof(val)) != 0)
+    {
+        return CHIP_ERROR_POSIX(errno);
+    }
 
-        // Enable keepalives for the connection.
-        val = 1; // enable
-        if (setsockopt(mSocket, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)) != 0)
-            return CHIP_ERROR_POSIX(errno);
+    // Enable keepalives for the connection.
+    val = 1; // enable
+    if (setsockopt(mSocket, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)) != 0)
+    {
+        return CHIP_ERROR_POSIX(errno);
     }
 
-    return res;
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR TCPEndPoint::DisableKeepAlive()
 {
-    CHIP_ERROR res = CHIP_NO_ERROR;
-
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
+    // Disable keepalives on the connection.
+    int val = 0; // disable
+    if (setsockopt(mSocket, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)) != 0)
     {
-        int val;
-
-        // Disable keepalives on the connection.
-        val = 0; // disable
-        if (setsockopt(mSocket, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)) != 0)
-            return CHIP_ERROR_POSIX(errno);
+        return CHIP_ERROR_POSIX(errno);
     }
 
-    return res;
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR TCPEndPoint::AckReceive(uint16_t len)
 {
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
     // nothing to do for sockets case
     return CHIP_NO_ERROR;
@@ -1920,7 +1873,7 @@ CHIP_ERROR TCPEndPoint::GetSocket(IPAddressType addrType)
 #endif // INET_CONFIG_ENABLE_IPV4
         else
             return INET_ERROR_WRONG_ADDRESS_TYPE;
-        mSocket = ::socket(family, SOCK_STREAM | SOCK_FLAGS, 0);
+        mSocket = ::socket(family, SOCK_STREAM | SOCK_CLOEXEC, 0);
         if (mSocket == -1)
             return CHIP_ERROR_POSIX(errno);
         ReturnErrorOnFailure(static_cast<System::LayerSockets *>(Layer().SystemLayer())->StartWatchingSocket(mSocket, &mWatch));
@@ -2011,7 +1964,7 @@ void TCPEndPoint::HandlePendingIO(System::SocketEvents events)
 
         // If in a state were receiving is allowed, and the app is ready to receive data, and data is ready
         // on the socket, receive inbound data from the connection.
-        if ((mState == State::kConnected || mState == State::kSendShutdown) && ReceiveEnabled && OnDataReceived != nullptr &&
+        if ((mState == State::kConnected || mState == State::kSendShutdown) && mReceiveEnabled && OnDataReceived != nullptr &&
             events.Has(System::SocketEventFlags::kRead))
             ReceiveData();
     }
@@ -2553,7 +2506,7 @@ void TCPEndPoint::Init(InetLayer * inetLayer)
 {
     InitEndPointBasis(*inetLayer);
 
-    ReceiveEnabled = true;
+    mReceiveEnabled = true;
 
     // Initialize to zero for using system defaults.
     mConnectTimeoutMsecs = 0;
@@ -2594,7 +2547,7 @@ void TCPEndPoint::DriveReceiving()
 {
     // If there's data in the receive queue and the app is ready to receive it then call the app's callback
     // with the entire receive queue.
-    if (!mRcvQueue.IsNull() && ReceiveEnabled && OnDataReceived != nullptr)
+    if (!mRcvQueue.IsNull() && mReceiveEnabled && OnDataReceived != nullptr)
     {
         // Acknowledgement is done after handling the buffers to allow the
         // application processing to throttle flow.
diff --git a/src/inet/TCPEndPoint.h b/src/inet/TCPEndPoint.h
index d11fd06ba187c4..0dcad952a21876 100644
--- a/src/inet/TCPEndPoint.h
+++ b/src/inet/TCPEndPoint.h
@@ -48,6 +48,7 @@ class TCPTest;
 namespace Inet {
 
 class InetLayer;
+class TCPTest;
 
 /**
  * @brief   Objects of this class represent TCP transport endpoints.
@@ -61,31 +62,9 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis
 {
     friend class InetLayer;
     friend class ::chip::Transport::TCPTest;
+    friend class TCPTest;
 
 public:
-    /** Control switch indicating whether the application is receiving data. */
-    bool ReceiveEnabled;
-
-    /**
-     * Basic dynamic state of the underlying endpoint.
-     *
-     *  Objects are initialized in the "ready" state, proceed to subsequent
-     *  states corresponding to a simplification of the states of the TCP
-     *  transport state machine.
-     */
-    enum class State : uint8_t
-    {
-        kReady           = 0, /**< Endpoint initialized, but not bound. */
-        kBound           = 1, /**< Endpoint bound, but not listening. */
-        kListening       = 2, /**< Endpoint receiving connections. */
-        kConnecting      = 3, /**< Endpoint attempting to connect. */
-        kConnected       = 4, /**< Endpoint connected, ready for tx/rx. */
-        kSendShutdown    = 5, /**< Endpoint initiated its half-close. */
-        kReceiveShutdown = 6, /**< Endpoint responded to half-close. */
-        kClosing         = 7, /**< Endpoint closing bidirectionally. */
-        kClosed          = 8  /**< Endpoint closed, ready for release. */
-    } mState;
-
     TCPEndPoint() = default;
 
     /**
@@ -174,7 +153,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis
      * @retval  CHIP_ERROR_CONNECTION_ABORTED   TCP connection no longer open.
      *
      * @details
-     *  Do not use \c NULL pointer values for either argument.
+     *  Do not use \c nullptr for either argument.
      */
     CHIP_ERROR GetPeerInfo(IPAddress * retAddr, uint16_t * retPort) const;
 
@@ -189,9 +168,9 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis
      * @retval  CHIP_ERROR_CONNECTION_ABORTED   TCP connection no longer open.
      *
      * @details
-     *  Do not use \c NULL pointer values for either argument.
+     *  Do not use \c nullptr for either argument.
      */
-    CHIP_ERROR GetLocalInfo(IPAddress * retAddr, uint16_t * retPort);
+    CHIP_ERROR GetLocalInfo(IPAddress * retAddr, uint16_t * retPort) const;
 
     /**
      * @brief   Extract the interface id of the TCP endpoint.
@@ -216,32 +195,27 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis
     CHIP_ERROR Send(chip::System::PacketBufferHandle && data, bool push = true);
 
     /**
-     * @brief   Disable reception.
+     * Disable reception.
      *
-     * @details
      *  Disable all event handlers. Data sent to an endpoint that disables
      *  reception will be acknowledged until the receive window is exhausted.
      */
-    void DisableReceive() { ReceiveEnabled = false; }
+    void DisableReceive() { mReceiveEnabled = false; }
 
     /**
-     * @brief   Enable reception.
+     * Enable reception.
      *
-     * @details
      *  Enable all event handlers. Data sent to an endpoint that disables
      *  reception will be acknowledged until the receive window is exhausted.
      */
     void EnableReceive()
     {
-        ReceiveEnabled = true;
+        mReceiveEnabled = true;
         DriveReceiving();
     }
 
     /**
-     *  @brief EnableNoDelay
-     *
-     *    Switch off nagle buffering algorithm in TCP by setting the
-     *    TCP_NODELAY socket options.
+     * Switch off Nagle buffering algorithm.
      */
     CHIP_ERROR EnableNoDelay();
 
@@ -599,6 +573,29 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis
 private:
     static chip::System::ObjectPool<TCPEndPoint, INET_CONFIG_NUM_TCP_ENDPOINTS> sPool;
 
+    /**
+     * Basic dynamic state of the underlying endpoint.
+     *
+     *  Objects are initialized in the "ready" state, proceed to subsequent
+     *  states corresponding to a simplification of the states of the TCP
+     *  transport state machine.
+     */
+    enum class State : uint8_t
+    {
+        kReady           = 0, /**< Endpoint initialized, but not bound. */
+        kBound           = 1, /**< Endpoint bound, but not listening. */
+        kListening       = 2, /**< Endpoint receiving connections. */
+        kConnecting      = 3, /**< Endpoint attempting to connect. */
+        kConnected       = 4, /**< Endpoint connected, ready for tx/rx. */
+        kSendShutdown    = 5, /**< Endpoint initiated its half-close. */
+        kReceiveShutdown = 6, /**< Endpoint responded to half-close. */
+        kClosing         = 7, /**< Endpoint closing bidirectionally. */
+        kClosed          = 8  /**< Endpoint closed, ready for release. */
+    } mState;
+
+    /** Control switch indicating whether the application is receiving data. */
+    bool mReceiveEnabled;
+
     chip::System::PacketBufferHandle mRcvQueue;
     chip::System::PacketBufferHandle mSendQueue;
 #if INET_TCP_IDLE_CHECK_INTERVAL > 0
@@ -657,7 +654,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis
 
 #endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
 
-    TCPEndPoint(const TCPEndPoint &); // not defined
+    TCPEndPoint(const TCPEndPoint &) = delete;
 
     void Init(InetLayer * inetLayer);
     CHIP_ERROR DriveSending();
@@ -716,6 +713,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis
 #endif // CHIP_SYSTEM_CONFIG_USE_LWIP
 
 #if CHIP_SYSTEM_CONFIG_USE_SOCKETS
+    CHIP_ERROR GetSocketInfo(int getname(int, sockaddr *, socklen_t *), IPAddress * retAddr, uint16_t * retPort) const;
     CHIP_ERROR GetSocket(IPAddressType addrType);
     void HandlePendingIO(System::SocketEvents events);
     void ReceiveData();
diff --git a/src/inet/tests/TestInetLayer.cpp b/src/inet/tests/TestInetLayer.cpp
index 61a54388d960ab..ee3ff88a3ef303 100644
--- a/src/inet/tests/TestInetLayer.cpp
+++ b/src/inet/tests/TestInetLayer.cpp
@@ -185,6 +185,22 @@ static OptionSet *       sToolOptionSets[] =
 };
 // clang-format on
 
+namespace chip {
+namespace Inet {
+
+class TCPTest
+{
+public:
+    static bool StateIsConnected(const TCPEndPoint * endPoint) { return endPoint->mState == TCPEndPoint::State::kConnected; }
+    static bool StateIsConnectedOrReceiveShutdown(const TCPEndPoint * endPoint)
+    {
+        return endPoint->mState == TCPEndPoint::State::kConnected || endPoint->mState == TCPEndPoint::State::kReceiveShutdown;
+    }
+};
+
+} // namespace Inet
+} // namespace chip
+
 static void CheckSucceededOrFailed(TestState & aTestState, bool & aOutSucceeded, bool & aOutFailed)
 {
     const TransferStats & lStats = aTestState.mStats;
@@ -570,7 +586,7 @@ static CHIP_ERROR HandleTCPDataReceived(TCPEndPoint * aEndPoint, PacketBufferHan
     VerifyOrExit(aEndPoint != nullptr, lStatus = CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrExit(!aBuffer.IsNull(), lStatus = CHIP_ERROR_INVALID_ARGUMENT);
 
-    if (aEndPoint->mState != TCPEndPoint::State::kConnected)
+    if (!TCPTest::StateIsConnected(aEndPoint))
     {
         lStatus = aEndPoint->SetReceivedDataForTesting(std::move(aBuffer));
         INET_FAIL_ERROR(lStatus, "TCPEndPoint::PutBackReceivedData failed");
@@ -654,33 +670,18 @@ static void HandleUDPReceiveError(IPEndPointBasis * aEndPoint, CHIP_ERROR aError
 
 static bool IsTransportReadyForSend()
 {
-    bool lStatus = false;
-
     if ((gOptFlags & kOptFlagUseUDPIP) == kOptFlagUseUDPIP)
     {
-        lStatus = (sUDPIPEndPoint != nullptr);
+        return (sUDPIPEndPoint != nullptr);
     }
-    else if ((gOptFlags & kOptFlagUseTCPIP) == kOptFlagUseTCPIP)
+
+    if ((gOptFlags & kOptFlagUseTCPIP) == kOptFlagUseTCPIP)
     {
-        if (sTCPIPEndPoint != nullptr)
-        {
-            if (sTCPIPEndPoint->PendingSendLength() == 0)
-            {
-                switch (sTCPIPEndPoint->mState)
-                {
-                case TCPEndPoint::State::kConnected:
-                case TCPEndPoint::State::kReceiveShutdown:
-                    lStatus = true;
-                    break;
-
-                default:
-                    break;
-                }
-            }
-        }
+        return (sTCPIPEndPoint != nullptr) && (sTCPIPEndPoint->PendingSendLength() == 0) &&
+            TCPTest::StateIsConnectedOrReceiveShutdown(sTCPIPEndPoint);
     }
 
-    return (lStatus);
+    return false;
 }
 
 static CHIP_ERROR PrepareTransportForSend()

From 640174538fad2afcb6ca7cd7d09202b9ddbbc2f5 Mon Sep 17 00:00:00 2001
From: Kevin Schoedel <kpschoedel@google.com>
Date: Mon, 25 Oct 2021 10:47:16 -0400
Subject: [PATCH 2/2] review

---
 src/inet/TCPEndPoint.cpp | 115 +++++++++++++++++++++++++++++----------
 1 file changed, 87 insertions(+), 28 deletions(-)

diff --git a/src/inet/TCPEndPoint.cpp b/src/inet/TCPEndPoint.cpp
index f9ecba7671545a..5681619011728c 100644
--- a/src/inet/TCPEndPoint.cpp
+++ b/src/inet/TCPEndPoint.cpp
@@ -624,11 +624,9 @@ void TCPEndPoint::DoCloseImpl(CHIP_ERROR err, State oldState)
 
 CHIP_ERROR TCPEndPoint::AckReceive(uint16_t len)
 {
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
     CHIP_ERROR res = CHIP_NO_ERROR;
 
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
-
     // Lock LwIP stack
     LOCK_TCPIP_CORE();
 
@@ -1265,7 +1263,9 @@ CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, Inter
         // The behavior when connecting to an IPv6 link-local address without specifying an outbound
         // interface is ambiguous. So prevent it in all cases.
         if (addr.IsIPv6LinkLocal())
+        {
             return INET_ERROR_WRONG_ADDRESS_TYPE;
+        }
     }
     else
     {
@@ -1366,7 +1366,9 @@ CHIP_ERROR TCPEndPoint::ConnectImpl(const IPAddress & addr, uint16_t port, Inter
         // Wait for ability to read on this endpoint.
         ReturnErrorOnFailure(static_cast<System::LayerSockets *>(Layer().SystemLayer())->RequestCallbackOnPendingRead(mWatch));
         if (OnConnectComplete != nullptr)
+        {
             OnConnectComplete(this, CHIP_NO_ERROR);
+        }
     }
     else
     {
@@ -1422,8 +1424,7 @@ CHIP_ERROR TCPEndPoint::GetSocketInfo(int getname(int, sockaddr *, socklen_t *),
 
 CHIP_ERROR TCPEndPoint::GetInterfaceId(InterfaceId * retInterface)
 {
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
     SockAddr sa;
     memset(&sa, 0, sizeof(sa));
@@ -1550,7 +1551,9 @@ CHIP_ERROR TCPEndPoint::SetUserTimeoutImpl(uint32_t userTimeoutMillis)
     // Set the user timeout
     uint32_t val = userTimeoutMillis;
     if (setsockopt(mSocket, TCP_SOCKOPT_LEVEL, TCP_USER_TIMEOUT, &val, sizeof(val)) != 0)
+    {
         return CHIP_ERROR_POSIX(errno);
+    }
     return CHIP_NO_ERROR;
 #else  // TCP_USER_TIMEOUT
     return CHIP_ERROR_NOT_IMPLEMENTED;
@@ -1591,7 +1594,9 @@ CHIP_ERROR TCPEndPoint::DriveSendingImpl()
         if (lenSentRaw == -1)
         {
             if (errno != EAGAIN && errno != EWOULDBLOCK)
+            {
                 err = (errno == EPIPE) ? INET_ERROR_PEER_DISCONNECTED : CHIP_ERROR_POSIX(errno);
+            }
             break;
         }
 
@@ -1626,7 +1631,9 @@ CHIP_ERROR TCPEndPoint::DriveSendingImpl()
         }
 
         if (OnDataSent != nullptr)
+        {
             OnDataSent(this, lenSent);
+        }
 
 #if INET_CONFIG_ENABLE_TCP_SEND_IDLE_CALLBACKS
         // TCP Send is not Idle; Set state and notify if needed
@@ -1662,7 +1669,9 @@ CHIP_ERROR TCPEndPoint::DriveSendingImpl()
 #endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
 
         if (lenSent < bufLen)
+        {
             break;
+        }
     }
 
     if (err == CHIP_NO_ERROR)
@@ -1671,7 +1680,9 @@ CHIP_ERROR TCPEndPoint::DriveSendingImpl()
         if (mState == State::kSendShutdown && mSendQueue.IsNull())
         {
             if (shutdown(mSocket, SHUT_WR) != 0)
+            {
                 err = CHIP_ERROR_POSIX(errno);
+            }
         }
     }
 
@@ -1712,7 +1723,9 @@ void TCPEndPoint::DoCloseImpl(CHIP_ERROR err, State oldState)
                 lingerStruct.l_linger = 0;
 
                 if (setsockopt(mSocket, SOL_SOCKET, SO_LINGER, &lingerStruct, sizeof(lingerStruct)) != 0)
+                {
                     ChipLogError(Inet, "SO_LINGER: %d", errno);
+                }
             }
 
             static_cast<System::LayerSockets *>(Layer().SystemLayer())->StopWatchingSocket(&mWatch);
@@ -1866,16 +1879,24 @@ CHIP_ERROR TCPEndPoint::GetSocket(IPAddressType addrType)
     {
         int family;
         if (addrType == IPAddressType::kIPv6)
+        {
             family = PF_INET6;
 #if INET_CONFIG_ENABLE_IPV4
+        }
         else if (addrType == IPAddressType::kIPv4)
+        {
             family = PF_INET;
 #endif // INET_CONFIG_ENABLE_IPV4
+        }
         else
+        {
             return INET_ERROR_WRONG_ADDRESS_TYPE;
+        }
         mSocket = ::socket(family, SOCK_STREAM | SOCK_CLOEXEC, 0);
         if (mSocket == -1)
+        {
             return CHIP_ERROR_POSIX(errno);
+        }
         ReturnErrorOnFailure(static_cast<System::LayerSockets *>(Layer().SystemLayer())->StartWatchingSocket(mSocket, &mWatch));
         mAddrType = addrType;
 
@@ -1942,7 +1963,9 @@ void TCPEndPoint::HandlePendingIO(System::SocketEvents events)
             int osConRes;
             socklen_t optLen = sizeof(osConRes);
             if (getsockopt(mSocket, SOL_SOCKET, SO_ERROR, &osConRes, &optLen) != 0)
+            {
                 osConRes = errno;
+            }
 #else
             // On Mbed OS, connect blocks and never returns EINPROGRESS
             // The socket option SO_ERROR is not available.
@@ -1960,13 +1983,17 @@ void TCPEndPoint::HandlePendingIO(System::SocketEvents events)
         // If in a state where sending is allowed, and there is data to be sent, and the socket is ready for
         // writing, drive outbound data into the connection.
         if (IsConnected() && !mSendQueue.IsNull() && events.Has(System::SocketEventFlags::kWrite))
+        {
             DriveSending();
+        }
 
         // If in a state were receiving is allowed, and the app is ready to receive data, and data is ready
         // on the socket, receive inbound data from the connection.
         if ((mState == State::kConnected || mState == State::kSendShutdown) && mReceiveEnabled && OnDataReceived != nullptr &&
             events.Has(System::SocketEventFlags::kRead))
+        {
             ReceiveData();
+        }
     }
 
     Release();
@@ -1978,7 +2005,9 @@ void TCPEndPoint::ReceiveData()
     bool isNewBuf = true;
 
     if (mRcvQueue.IsNull())
+    {
         rcvBuf = System::PacketBufferHandle::New(kMaxReceiveMessageSize, 0);
+    }
     else
     {
         rcvBuf = mRcvQueue->Last();
@@ -2068,14 +2097,20 @@ void TCPEndPoint::ReceiveData()
             // the peer has closed. If no OnPeerClose is provided, we assume that the app
             // wants to close both directions and automatically enter the Closing state.
             if (mState == State::kConnected && OnPeerClose != nullptr)
+            {
                 mState = State::kReceiveShutdown;
+            }
             else
+            {
                 mState = State::kClosing;
+            }
             // Do not wait for ability to read on this endpoint.
             (void) static_cast<System::LayerSockets *>(Layer().SystemLayer())->ClearCallbackOnPendingRead(mWatch);
             // Call the app's OnPeerClose.
             if (OnPeerClose != nullptr)
+            {
                 OnPeerClose(this);
+            }
         }
 
         // Otherwise, add the new data onto the receive queue.
@@ -2089,9 +2124,13 @@ void TCPEndPoint::ReceiveData()
                 rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength));
                 rcvBuf.RightSize();
                 if (mRcvQueue.IsNull())
+                {
                     mRcvQueue = std::move(rcvBuf);
+                }
                 else
+                {
                     mRcvQueue->AddToEnd(std::move(rcvBuf));
+                }
             }
             else
             {
@@ -2131,7 +2170,9 @@ void TCPEndPoint::HandleIncomingConnection()
 
     // If there's no callback available, fail with an error.
     if (err == CHIP_NO_ERROR && OnConnectionReceived == nullptr)
+    {
         err = CHIP_ERROR_NO_CONNECTION_HANDLER;
+    }
 
     // Extract the peer's address information.
     if (err == CHIP_NO_ERROR)
@@ -2149,7 +2190,9 @@ void TCPEndPoint::HandleIncomingConnection()
         }
 #endif // INET_CONFIG_ENABLE_IPV4
         else
+        {
             err = CHIP_ERROR_INCORRECT_STATE;
+        }
     }
 
     // Attempt to allocate an end point object.
@@ -2194,7 +2237,9 @@ void TCPEndPoint::HandleIncomingConnection()
 
     // Otherwise immediately close the connection, clean up and call the app's error callback.
     if (conSocket != -1)
+    {
         close(conSocket);
+    }
     if (conEP != nullptr)
     {
         if (conEP->mState == State::kConnected)
@@ -2204,7 +2249,9 @@ void TCPEndPoint::HandleIncomingConnection()
         conEP->Release();
     }
     if (OnAcceptError != nullptr)
+    {
         OnAcceptError(this, err);
+    }
 }
 
 #if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
@@ -2259,13 +2306,13 @@ CHIP_ERROR TCPEndPoint::CheckConnectionProgress(bool & isProgressing)
 
 CHIP_ERROR TCPEndPoint::Bind(IPAddressType addrType, const IPAddress & addr, uint16_t port, bool reuseAddr)
 {
+    VerifyOrReturnError(mState == State::kReady, CHIP_ERROR_INCORRECT_STATE);
     CHIP_ERROR res = CHIP_NO_ERROR;
 
-    if (mState != State::kReady)
-        return CHIP_ERROR_INCORRECT_STATE;
-
     if (addr != IPAddress::Any && addr.Type() != IPAddressType::kAny && addr.Type() != addrType)
+    {
         return INET_ERROR_WRONG_ADDRESS_TYPE;
+    }
 
     res = BindImpl(addrType, addr, port, reuseAddr);
 
@@ -2279,11 +2326,9 @@ CHIP_ERROR TCPEndPoint::Bind(IPAddressType addrType, const IPAddress & addr, uin
 
 CHIP_ERROR TCPEndPoint::Listen(uint16_t backlog)
 {
+    VerifyOrReturnError(mState == State::kBound, CHIP_ERROR_INCORRECT_STATE);
     CHIP_ERROR res = CHIP_NO_ERROR;
 
-    if (mState != State::kBound)
-        return CHIP_ERROR_INCORRECT_STATE;
-
     res = ListenImpl(backlog);
 
     if (res == CHIP_NO_ERROR)
@@ -2299,11 +2344,9 @@ CHIP_ERROR TCPEndPoint::Listen(uint16_t backlog)
 
 CHIP_ERROR TCPEndPoint::Connect(const IPAddress & addr, uint16_t port, InterfaceId intfId)
 {
+    VerifyOrReturnError(mState == State::kReady || mState == State::kBound, CHIP_ERROR_INCORRECT_STATE);
     CHIP_ERROR res = CHIP_NO_ERROR;
 
-    if (mState != State::kReady && mState != State::kBound)
-        return CHIP_ERROR_INCORRECT_STATE;
-
     ReturnErrorOnFailure(ConnectImpl(addr, port, intfId));
 
     StartConnectTimerIfSet();
@@ -2313,12 +2356,9 @@ CHIP_ERROR TCPEndPoint::Connect(const IPAddress & addr, uint16_t port, Interface
 
 CHIP_ERROR TCPEndPoint::Send(System::PacketBufferHandle && data, bool push)
 {
+    VerifyOrReturnError(mState == State::kConnected || mState == State::kReceiveShutdown, CHIP_ERROR_INCORRECT_STATE);
     CHIP_ERROR res = CHIP_NO_ERROR;
 
-    if (mState != State::kConnected && mState != State::kReceiveShutdown)
-    {
-        return CHIP_ERROR_INCORRECT_STATE;
-    }
     bool queueWasEmpty = mSendQueue.IsNull();
     if (queueWasEmpty)
     {
@@ -2332,15 +2372,16 @@ CHIP_ERROR TCPEndPoint::Send(System::PacketBufferHandle && data, bool push)
     ReturnErrorOnFailure(SendQueuedImpl(queueWasEmpty));
 
     if (push)
+    {
         res = DriveSending();
+    }
 
     return res;
 }
 
 CHIP_ERROR TCPEndPoint::SetReceivedDataForTesting(System::PacketBufferHandle && data)
 {
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
     mRcvQueue = std::move(data);
 
@@ -2350,24 +2391,26 @@ CHIP_ERROR TCPEndPoint::SetReceivedDataForTesting(System::PacketBufferHandle &&
 uint32_t TCPEndPoint::PendingSendLength()
 {
     if (!mSendQueue.IsNull())
+    {
         return mSendQueue->TotalLength();
+    }
     return 0;
 }
 
 uint32_t TCPEndPoint::PendingReceiveLength()
 {
     if (!mRcvQueue.IsNull())
+    {
         return mRcvQueue->TotalLength();
+    }
     return 0;
 }
 
 CHIP_ERROR TCPEndPoint::Shutdown()
 {
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
     CHIP_ERROR err = CHIP_NO_ERROR;
 
-    if (!IsConnected())
-        return CHIP_ERROR_INCORRECT_STATE;
-
     // If fully connected, enter the SendShutdown state.
     if (mState == State::kConnected)
     {
@@ -2377,7 +2420,9 @@ CHIP_ERROR TCPEndPoint::Shutdown()
 
     // Otherwise, if the peer has already closed their end of the connection,
     else if (mState == State::kReceiveShutdown)
+    {
         err = DoClose(err, false);
+    }
 
     return err;
 }
@@ -2422,7 +2467,9 @@ void TCPEndPoint::Free()
     // Ensure the end point is Closed or Closing.
     err = Close();
     if (err != CHIP_NO_ERROR)
+    {
         Abort();
+    }
 
     // Release the Retain() that happened when the end point was allocated
     // [on LwIP, the object may still be alive if DoClose() used the
@@ -2438,7 +2485,9 @@ void TCPEndPoint::SetIdleTimeout(uint32_t timeoutMS)
     bool isIdleTimerRunning = lInetLayer.IsIdleTimerRunning();
 
     if (newIdleTimeout > UINT16_MAX)
+    {
         newIdleTimeout = UINT16_MAX;
+    }
     mIdleTimeout = mRemainingIdleTime = static_cast<uint16_t>(newIdleTimeout);
 
     if (!isIdleTimerRunning && mIdleTimeout)
@@ -2451,13 +2500,9 @@ void TCPEndPoint::SetIdleTimeout(uint32_t timeoutMS)
 
 CHIP_ERROR TCPEndPoint::SetUserTimeout(uint32_t userTimeoutMillis)
 {
+    VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
     CHIP_ERROR res = CHIP_NO_ERROR;
 
-    if (!IsConnected())
-    {
-        return CHIP_ERROR_INCORRECT_STATE;
-    }
-
 #if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
 
     // Store the User timeout configuration if it is being overridden.
@@ -2536,7 +2581,9 @@ CHIP_ERROR TCPEndPoint::DriveSending()
     CHIP_ERROR err = DriveSendingImpl();
 
     if (err != CHIP_NO_ERROR)
+    {
         DoClose(err, false);
+    }
 
     CHIP_SYSTEM_FAULT_INJECT_ASYNC_EVENT();
 
@@ -2564,7 +2611,9 @@ void TCPEndPoint::DriveReceiving()
     // If the connection is closing, and the receive queue is now empty, call DoClose() to complete
     // the process of closing the connection.
     if (mState == State::kClosing && mRcvQueue.IsNull())
+    {
         DoClose(CHIP_NO_ERROR, false);
+    }
 }
 
 void TCPEndPoint::HandleConnectComplete(CHIP_ERROR err)
@@ -2583,7 +2632,9 @@ void TCPEndPoint::HandleConnectComplete(CHIP_ERROR err)
         HandleConnectCompleteImpl();
 
         if (OnConnectComplete != nullptr)
+        {
             OnConnectComplete(this, CHIP_NO_ERROR);
+        }
     }
 
     // Otherwise, close the connection with an error.
@@ -2603,9 +2654,13 @@ CHIP_ERROR TCPEndPoint::DoClose(CHIP_ERROR err, bool suppressCallback)
     // ... THEN enter the Closing state, allowing the queued data to drain,
     // ... OTHERWISE go straight to the Closed state.
     if (IsConnected() && err == CHIP_NO_ERROR && (!mSendQueue.IsNull() || !mRcvQueue.IsNull()))
+    {
         mState = State::kClosing;
+    }
     else
+    {
         mState = State::kClosed;
+    }
 
     if (oldState != State::kClosed)
     {
@@ -2639,12 +2694,16 @@ CHIP_ERROR TCPEndPoint::DoClose(CHIP_ERROR err, bool suppressCallback)
             if (oldState == State::kConnecting)
             {
                 if (OnConnectComplete != nullptr)
+                {
                     OnConnectComplete(this, err);
+                }
             }
             else if ((oldState == State::kConnected || oldState == State::kSendShutdown || oldState == State::kReceiveShutdown ||
                       oldState == State::kClosing) &&
                      OnConnectionClosed != nullptr)
+            {
                 OnConnectionClosed(this, err);
+            }
         }
 
         // Decrement the ref count that was added when the connection started (in Connect()) or listening started (in Listen()).