From 4797999c542340b855ea3a44c8f845f560c9ddc6 Mon Sep 17 00:00:00 2001 From: Kevin Schoedel <67607049+kpschoedel@users.noreply.github.com> Date: Wed, 10 Nov 2021 10:20:10 -0500 Subject: [PATCH] Inet: Use constructors for Inet EndPoints (#11584) * Inet: Use constructors for Inet EndPoints #### Problem `Inet::TCPEndPoint` and `Inet::UDPEndPoint` historically could not use constructors because of `System::ObjectPool` limitations. Incidentally renamed `mAppState` for consistency (it had been in `System::Object` prior to #11428). This is a step toward #7715 _Virtualize System and Inet interfaces_, split off to reduce the complexity of an upcoming PR. #### Change overview Convert from `Init()` to constructors. Transitionally, the constructors still call a per-implementation function `InitImpl()`. #### Testing CI; no changes to functionality. * explicitly initialize all members --- src/inet/EndPointBasis.h | 30 ++++---------------- src/inet/EndPointBasisImplLwIP.h | 5 ++-- src/inet/EndPointBasisImplNetworkFramework.h | 2 +- src/inet/EndPointBasisImplSockets.h | 4 ++- src/inet/InetLayer.cpp | 12 ++++---- src/inet/TCPEndPoint.cpp | 29 ------------------- src/inet/TCPEndPoint.h | 25 +++++++++++++--- src/inet/UDPEndPoint.cpp | 8 +----- src/inet/UDPEndPoint.h | 8 ++++-- src/lib/dnssd/minimal_mdns/Server.cpp | 2 +- src/transport/raw/TCP.cpp | 16 +++++------ src/transport/raw/UDP.cpp | 2 +- 12 files changed, 55 insertions(+), 88 deletions(-) diff --git a/src/inet/EndPointBasis.h b/src/inet/EndPointBasis.h index c1d503d8eec883..d7791815052732 100644 --- a/src/inet/EndPointBasis.h +++ b/src/inet/EndPointBasis.h @@ -38,36 +38,18 @@ class InetLayer; class DLL_EXPORT EndPointBase { public: - /** - * Returns a reference to the Inet layer object that owns this basis object. - */ - InetLayer & Layer() const { return *mInetLayer; } + EndPointBase(InetLayer & aInetLayer, void * aAppState = nullptr) : mAppState(aAppState), mInetLayer(aInetLayer) {} + virtual ~EndPointBase() = default; /** - * Returns \c true if the basis object was obtained by the specified Inet layer instance. - * - * @note - * Does not check whether the object is actually obtained by the system layer instance associated with the Inet layer - * instance. It merely tests whether \c aInetLayer is the Inet layer instance that was provided to \c InitInetLayerBasis. + * Returns a reference to the Inet layer object that owns this basis object. */ - bool IsCreatedByInetLayer(const InetLayer & aInetLayer) const { return mInetLayer == &aInetLayer; } + InetLayer & Layer() const { return mInetLayer; } - void * AppState; + void * mAppState; private: - InetLayer * mInetLayer; /**< Pointer to the InetLayer object that owns this object. */ - -protected: - virtual ~EndPointBase() = default; - - void InitEndPointBasis(InetLayer & aInetLayer, void * aAppState = nullptr) - { - AppState = aAppState; - mInetLayer = &aInetLayer; - InitEndPointBasisImpl(); - } - - virtual void InitEndPointBasisImpl() = 0; + InetLayer & mInetLayer; /**< InetLayer object that owns this object. */ }; } // namespace Inet diff --git a/src/inet/EndPointBasisImplLwIP.h b/src/inet/EndPointBasisImplLwIP.h index e8df0ed4baed1b..9d398c2bf182d2 100644 --- a/src/inet/EndPointBasisImplLwIP.h +++ b/src/inet/EndPointBasisImplLwIP.h @@ -35,8 +35,9 @@ namespace Inet { class DLL_EXPORT EndPointImplLwIP : public EndPointBase { protected: - // EndPointBase overrides - void InitEndPointBasisImpl() override { mLwIPEndPointType = LwIPEndPointType::Unknown; } + EndPointImplLwIP(InetLayer & inetLayer, void * appState = nullptr) : + EndPointBase(inetLayer, appState), mLwIPEndPointType(LwIPEndPointType::Unknown) + {} /** Encapsulated LwIP protocol control block */ union diff --git a/src/inet/EndPointBasisImplNetworkFramework.h b/src/inet/EndPointBasisImplNetworkFramework.h index b38344cb472600..8b72c18442fc44 100644 --- a/src/inet/EndPointBasisImplNetworkFramework.h +++ b/src/inet/EndPointBasisImplNetworkFramework.h @@ -34,7 +34,7 @@ namespace Inet { class DLL_EXPORT EndPointImplNetworkFramework : public EndPointBase { protected: - void InitEndPointBasisImpl() override {} + EndPointImplNetworkFramework(InetLayer & inetLayer, void * appState = nullptr) : EndPointBase(inetLayer, appState) {} nw_parameters_t mParameters; IPAddressType mAddrType; /**< Protocol family, i.e. IPv4 or IPv6. */ diff --git a/src/inet/EndPointBasisImplSockets.h b/src/inet/EndPointBasisImplSockets.h index d4db35cebc7dec..f6c4c474541d57 100644 --- a/src/inet/EndPointBasisImplSockets.h +++ b/src/inet/EndPointBasisImplSockets.h @@ -33,7 +33,9 @@ namespace Inet { class DLL_EXPORT EndPointImplSockets : public EndPointBase { protected: - void InitEndPointBasisImpl() override { mSocket = kInvalidSocketFd; } + EndPointImplSockets(InetLayer & inetLayer, void * appState = nullptr) : + EndPointBase(inetLayer, appState), mSocket(kInvalidSocketFd) + {} static constexpr int kInvalidSocketFd = -1; int mSocket; /**< Encapsulated socket descriptor. */ diff --git a/src/inet/InetLayer.cpp b/src/inet/InetLayer.cpp index f5e282d37a62e8..54436d62e87d8a 100644 --- a/src/inet/InetLayer.cpp +++ b/src/inet/InetLayer.cpp @@ -250,7 +250,7 @@ CHIP_ERROR InetLayer::Shutdown() #if INET_CONFIG_ENABLE_TCP_ENDPOINT // Abort all TCP endpoints owned by this instance. TCPEndPoint::sPool.ForEachActiveObject([&](TCPEndPoint * lEndPoint) { - if ((lEndPoint != nullptr) && lEndPoint->IsCreatedByInetLayer(*this)) + if ((lEndPoint != nullptr) && &lEndPoint->Layer() == this) { lEndPoint->Abort(); } @@ -261,7 +261,7 @@ CHIP_ERROR InetLayer::Shutdown() #if INET_CONFIG_ENABLE_UDP_ENDPOINT // Close all UDP endpoints owned by this instance. UDPEndPoint::sPool.ForEachActiveObject([&](UDPEndPoint * lEndPoint) { - if ((lEndPoint != nullptr) && lEndPoint->IsCreatedByInetLayer(*this)) + if ((lEndPoint != nullptr) && &lEndPoint->Layer() == this) { lEndPoint->Close(); } @@ -429,14 +429,13 @@ CHIP_ERROR InetLayer::NewTCPEndPoint(TCPEndPoint ** retEndPoint) VerifyOrReturnError(mLayerState.IsInitialized(), CHIP_ERROR_INCORRECT_STATE); - *retEndPoint = TCPEndPoint::sPool.CreateObject(); + *retEndPoint = TCPEndPoint::sPool.CreateObject(*this); if (*retEndPoint == nullptr) { ChipLogError(Inet, "%s endpoint pool FULL", "TCP"); return CHIP_ERROR_ENDPOINT_POOL_FULL; } - (*retEndPoint)->Init(this); SYSTEM_STATS_INCREMENT(chip::System::Stats::kInetLayer_NumTCPEps); return CHIP_NO_ERROR; @@ -469,14 +468,13 @@ CHIP_ERROR InetLayer::NewUDPEndPoint(UDPEndPoint ** retEndPoint) VerifyOrReturnError(mLayerState.IsInitialized(), CHIP_ERROR_INCORRECT_STATE); - *retEndPoint = UDPEndPoint::sPool.CreateObject(); + *retEndPoint = UDPEndPoint::sPool.CreateObject(*this); if (*retEndPoint == nullptr) { ChipLogError(Inet, "%s endpoint pool FULL", "UDP"); return CHIP_ERROR_ENDPOINT_POOL_FULL; } - (*retEndPoint)->Init(this); SYSTEM_STATS_INCREMENT(chip::System::Stats::kInetLayer_NumUDPEps); return CHIP_NO_ERROR; @@ -558,7 +556,7 @@ void InetLayer::HandleTCPInactivityTimer(chip::System::Layer * aSystemLayer, voi bool lTimerRequired = lInetLayer.IsIdleTimerRunning(); TCPEndPoint::sPool.ForEachActiveObject([&](TCPEndPoint * lEndPoint) { - if (!lEndPoint->IsCreatedByInetLayer(lInetLayer)) + if (&lEndPoint->Layer() != &lInetLayer) return true; if (!lEndPoint->IsConnected()) return true; diff --git a/src/inet/TCPEndPoint.cpp b/src/inet/TCPEndPoint.cpp index f0b5fb207fad71..f93f68805c5d71 100644 --- a/src/inet/TCPEndPoint.cpp +++ b/src/inet/TCPEndPoint.cpp @@ -2594,35 +2594,6 @@ bool TCPEndPoint::IsConnected(State state) state == State::kClosing; } -void TCPEndPoint::Init(InetLayer * inetLayer) -{ - InitEndPointBasis(*inetLayer); - - mReceiveEnabled = true; - - // Initialize to zero for using system defaults. - mConnectTimeoutMsecs = 0; - -#if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT - mUserTimeoutMillis = INET_CONFIG_DEFAULT_TCP_USER_TIMEOUT_MSEC; - - mUserTimeoutTimerRunning = false; - -#if INET_CONFIG_ENABLE_TCP_SEND_IDLE_CALLBACKS - mIsTCPSendIdle = true; - - mTCPSendQueuePollPeriodMillis = INET_CONFIG_TCP_SEND_QUEUE_POLL_INTERVAL_MSEC; - - mTCPSendQueueRemainingPollCount = MaxTCPSendQueuePolls(); - - OnTCPSendIdleChanged = NULL; -#endif // INET_CONFIG_ENABLE_TCP_SEND_IDLE_CALLBACKS - -#endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT - - InitImpl(); -} - CHIP_ERROR TCPEndPoint::DriveSending() { CHIP_ERROR err = DriveSendingImpl(); diff --git a/src/inet/TCPEndPoint.h b/src/inet/TCPEndPoint.h index a6f05744df2685..41244c669dd5c2 100644 --- a/src/inet/TCPEndPoint.h +++ b/src/inet/TCPEndPoint.h @@ -74,7 +74,24 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis, public ReferenceCounted mTCPSendQueuePollPeriodMillis) ? (mUserTimeoutMillis / mTCPSendQueuePollPeriodMillis) : 1; + return (userTimeout > pollPeriod) ? (userTimeout / pollPeriod) : 1; } + uint16_t MaxTCPSendQueuePolls(void) { return MaxTCPSendQueuePolls(mUserTimeoutMillis, mTCPSendQueuePollPeriodMillis); } #endif // INET_CONFIG_ENABLE_TCP_SEND_IDLE_CALLBACKS #if CHIP_SYSTEM_CONFIG_USE_SOCKETS @@ -666,7 +684,6 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis, public ReferenceCounted { public: - UDPEndPoint() = default; + UDPEndPoint(InetLayer & inetLayer, void * appState = nullptr) : + EndPointBasis(inetLayer, appState), mState(State::kReady), OnMessageReceived(nullptr), OnReceiveError(nullptr) + { + InitImpl(); + } UDPEndPoint(const UDPEndPoint &) = delete; UDPEndPoint(UDPEndPoint &&) = delete; @@ -262,8 +266,6 @@ class DLL_EXPORT UDPEndPoint : public EndPointBasis, public ReferenceCounted(endPoint->AppState); + ServerBase * srv = static_cast(endPoint->mAppState); if (!srv->mDelegate) { return; diff --git a/src/transport/raw/TCP.cpp b/src/transport/raw/TCP.cpp index 4319b871811f1e..98a1b45865cd20 100644 --- a/src/transport/raw/TCP.cpp +++ b/src/transport/raw/TCP.cpp @@ -98,7 +98,7 @@ CHIP_ERROR TCPBase::Init(TcpListenParameters & params) err = mListenSocket->Listen(kListenBacklogSize); SuccessOrExit(err); - mListenSocket->AppState = reinterpret_cast(this); + mListenSocket->mAppState = reinterpret_cast(this); mListenSocket->OnDataReceived = OnTcpReceive; mListenSocket->OnConnectComplete = OnConnectionComplete; mListenSocket->OnConnectionClosed = OnConnectionClosed; @@ -254,7 +254,7 @@ CHIP_ERROR TCPBase::SendAfterConnect(const PeerAddress & addr, System::PacketBuf #endif SuccessOrExit(err); - endPoint->AppState = reinterpret_cast(this); + endPoint->mAppState = reinterpret_cast(this); endPoint->OnDataReceived = OnTcpReceive; endPoint->OnConnectComplete = OnConnectionComplete; endPoint->OnConnectionClosed = OnConnectionClosed; @@ -364,7 +364,7 @@ CHIP_ERROR TCPBase::OnTcpReceive(Inet::TCPEndPoint * endPoint, System::PacketBuf endPoint->GetInterfaceId(&interfaceId); PeerAddress peerAddress = PeerAddress::TCP(ipAddress, port, interfaceId); - TCPBase * tcp = reinterpret_cast(endPoint->AppState); + TCPBase * tcp = reinterpret_cast(endPoint->mAppState); CHIP_ERROR err = tcp->ProcessReceivedBuffer(endPoint, peerAddress, std::move(buffer)); if (err != CHIP_NO_ERROR) @@ -380,7 +380,7 @@ void TCPBase::OnConnectionComplete(Inet::TCPEndPoint * endPoint, CHIP_ERROR inet { CHIP_ERROR err = CHIP_NO_ERROR; bool foundPendingPacket = false; - TCPBase * tcp = reinterpret_cast(endPoint->AppState); + TCPBase * tcp = reinterpret_cast(endPoint->mAppState); Inet::IPAddress ipAddress; uint16_t port; Inet::InterfaceId interfaceId; @@ -452,7 +452,7 @@ void TCPBase::OnConnectionComplete(Inet::TCPEndPoint * endPoint, CHIP_ERROR inet void TCPBase::OnConnectionClosed(Inet::TCPEndPoint * endPoint, CHIP_ERROR err) { - TCPBase * tcp = reinterpret_cast(endPoint->AppState); + TCPBase * tcp = reinterpret_cast(endPoint->mAppState); ChipLogProgress(Inet, "Connection closed."); @@ -470,7 +470,7 @@ void TCPBase::OnConnectionClosed(Inet::TCPEndPoint * endPoint, CHIP_ERROR err) void TCPBase::OnConnectionReceived(Inet::TCPEndPoint * listenEndPoint, Inet::TCPEndPoint * endPoint, const Inet::IPAddress & peerAddress, uint16_t peerPort) { - TCPBase * tcp = reinterpret_cast(listenEndPoint->AppState); + TCPBase * tcp = reinterpret_cast(listenEndPoint->mAppState); if (tcp->mUsedEndPointCount < tcp->mActiveConnectionsSize) { @@ -484,7 +484,7 @@ void TCPBase::OnConnectionReceived(Inet::TCPEndPoint * listenEndPoint, Inet::TCP } } - endPoint->AppState = listenEndPoint->AppState; + endPoint->mAppState = listenEndPoint->mAppState; endPoint->OnDataReceived = OnTcpReceive; endPoint->OnConnectComplete = OnConnectionComplete; endPoint->OnConnectionClosed = OnConnectionClosed; @@ -531,7 +531,7 @@ void TCPBase::Disconnect(const PeerAddress & address) void TCPBase::OnPeerClosed(Inet::TCPEndPoint * endPoint) { - TCPBase * tcp = reinterpret_cast(endPoint->AppState); + TCPBase * tcp = reinterpret_cast(endPoint->mAppState); for (size_t i = 0; i < tcp->mActiveConnectionsSize; i++) { diff --git a/src/transport/raw/UDP.cpp b/src/transport/raw/UDP.cpp index 88f7546ab642cb..5915418e9dc42a 100644 --- a/src/transport/raw/UDP.cpp +++ b/src/transport/raw/UDP.cpp @@ -114,7 +114,7 @@ CHIP_ERROR UDP::SendMessage(const Transport::PeerAddress & address, System::Pack void UDP::OnUdpReceive(Inet::UDPEndPoint * endPoint, System::PacketBufferHandle && buffer, const Inet::IPPacketInfo * pktInfo) { CHIP_ERROR err = CHIP_NO_ERROR; - UDP * udp = reinterpret_cast(endPoint->AppState); + UDP * udp = reinterpret_cast(endPoint->mAppState); PeerAddress peerAddress = PeerAddress::UDP(pktInfo->SrcAddress, pktInfo->SrcPort, pktInfo->Interface); udp->HandleMessageReceived(peerAddress, std::move(buffer));