Skip to content

Commit

Permalink
Inet: Use constructors for Inet EndPoints (#11584)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
kpschoedel authored and pull[bot] committed Nov 14, 2023
1 parent 6e447e2 commit 57ade85
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 88 deletions.
30 changes: 6 additions & 24 deletions src/inet/EndPointBasis.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/inet/EndPointBasisImplLwIP.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/inet/EndPointBasisImplNetworkFramework.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
4 changes: 3 additions & 1 deletion src/inet/EndPointBasisImplSockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
12 changes: 5 additions & 7 deletions src/inet/InetLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
29 changes: 0 additions & 29 deletions src/inet/TCPEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
25 changes: 21 additions & 4 deletions src/inet/TCPEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,24 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis, public ReferenceCounted<TCP
friend class TCPTest;

public:
TCPEndPoint() = default;
TCPEndPoint(InetLayer & inetLayer, void * appState = nullptr) :
EndPointBasis(inetLayer, appState), OnConnectComplete(nullptr), OnDataReceived(nullptr), OnDataSent(nullptr),
OnConnectionClosed(nullptr), OnPeerClose(nullptr), OnConnectionReceived(nullptr), OnAcceptError(nullptr),
mState(State::kReady), mReceiveEnabled(true), mConnectTimeoutMsecs(0) // Initialize to zero for using system defaults.
#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(INET_CONFIG_DEFAULT_TCP_USER_TIMEOUT_MSEC, INET_CONFIG_TCP_SEND_QUEUE_POLL_INTERVAL_MSEC)),
OnTCPSendIdleChanged(nullptr)
#endif // INET_CONFIG_ENABLE_TCP_SEND_IDLE_CALLBACKS
#endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
{
InitImpl();
}

/**
* @brief Bind the endpoint to an interface IP address.
Expand Down Expand Up @@ -644,13 +661,14 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis, public ReferenceCounted<TCP
void ScheduleNextTCPUserTimeoutPoll(uint32_t aTimeOut);

#if INET_CONFIG_ENABLE_TCP_SEND_IDLE_CALLBACKS
uint16_t MaxTCPSendQueuePolls(void)
static constexpr uint16_t MaxTCPSendQueuePolls(uint16_t userTimeout, uint16_t pollPeriod)
{
// If the UserTimeout is configured less than or equal to the poll interval,
// return 1 to poll at least once instead of returning zero and timing out
// immediately.
return (mUserTimeoutMillis > 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
Expand All @@ -666,7 +684,6 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis, public ReferenceCounted<TCP

TCPEndPoint(const TCPEndPoint &) = delete;

void Init(InetLayer * inetLayer);
CHIP_ERROR DriveSending();
void DriveReceiving();
void HandleConnectComplete(CHIP_ERROR err);
Expand Down
8 changes: 1 addition & 7 deletions src/inet/UDPEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1988,7 +1988,7 @@ CHIP_ERROR UDPEndPoint::Listen(OnMessageReceivedFunct onMessageReceived, OnRecei

OnMessageReceived = onMessageReceived;
OnReceiveError = onReceiveError;
AppState = appState;
mAppState = appState;

ReturnErrorOnFailure(ListenImpl());

Expand Down Expand Up @@ -2028,12 +2028,6 @@ void UDPEndPoint::Close()
}
}

void UDPEndPoint::Init(InetLayer * inetLayer)
{
InitEndPointBasis(*inetLayer);
InitImpl();
}

CHIP_ERROR UDPEndPoint::JoinMulticastGroup(InterfaceId aInterfaceId, const IPAddress & aAddress)
{
CHIP_ERROR lRetval = CHIP_ERROR_NOT_IMPLEMENTED;
Expand Down
8 changes: 5 additions & 3 deletions src/inet/UDPEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ class UDPEndPointDeletor
class DLL_EXPORT UDPEndPoint : public EndPointBasis, public ReferenceCounted<UDPEndPoint, UDPEndPointDeletor>
{
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;
Expand Down Expand Up @@ -262,8 +266,6 @@ class DLL_EXPORT UDPEndPoint : public EndPointBasis, public ReferenceCounted<UDP
private:
friend class InetLayer;

void Init(InetLayer * inetLayer);

/**
* Basic dynamic state of the underlying endpoint.
*
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/minimal_mdns/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ CHIP_ERROR ServerBase::BroadcastSend(chip::System::PacketBufferHandle && data, u
void ServerBase::OnUdpPacketReceived(chip::Inet::UDPEndPoint * endPoint, chip::System::PacketBufferHandle && buffer,
const chip::Inet::IPPacketInfo * info)
{
ServerBase * srv = static_cast<ServerBase *>(endPoint->AppState);
ServerBase * srv = static_cast<ServerBase *>(endPoint->mAppState);
if (!srv->mDelegate)
{
return;
Expand Down
16 changes: 8 additions & 8 deletions src/transport/raw/TCP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ CHIP_ERROR TCPBase::Init(TcpListenParameters & params)
err = mListenSocket->Listen(kListenBacklogSize);
SuccessOrExit(err);

mListenSocket->AppState = reinterpret_cast<void *>(this);
mListenSocket->mAppState = reinterpret_cast<void *>(this);
mListenSocket->OnDataReceived = OnTcpReceive;
mListenSocket->OnConnectComplete = OnConnectionComplete;
mListenSocket->OnConnectionClosed = OnConnectionClosed;
Expand Down Expand Up @@ -254,7 +254,7 @@ CHIP_ERROR TCPBase::SendAfterConnect(const PeerAddress & addr, System::PacketBuf
#endif
SuccessOrExit(err);

endPoint->AppState = reinterpret_cast<void *>(this);
endPoint->mAppState = reinterpret_cast<void *>(this);
endPoint->OnDataReceived = OnTcpReceive;
endPoint->OnConnectComplete = OnConnectionComplete;
endPoint->OnConnectionClosed = OnConnectionClosed;
Expand Down Expand Up @@ -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<TCPBase *>(endPoint->AppState);
TCPBase * tcp = reinterpret_cast<TCPBase *>(endPoint->mAppState);
CHIP_ERROR err = tcp->ProcessReceivedBuffer(endPoint, peerAddress, std::move(buffer));

if (err != CHIP_NO_ERROR)
Expand All @@ -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<TCPBase *>(endPoint->AppState);
TCPBase * tcp = reinterpret_cast<TCPBase *>(endPoint->mAppState);
Inet::IPAddress ipAddress;
uint16_t port;
Inet::InterfaceId interfaceId;
Expand Down Expand Up @@ -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<TCPBase *>(endPoint->AppState);
TCPBase * tcp = reinterpret_cast<TCPBase *>(endPoint->mAppState);

ChipLogProgress(Inet, "Connection closed.");

Expand All @@ -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<TCPBase *>(listenEndPoint->AppState);
TCPBase * tcp = reinterpret_cast<TCPBase *>(listenEndPoint->mAppState);

if (tcp->mUsedEndPointCount < tcp->mActiveConnectionsSize)
{
Expand All @@ -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;
Expand Down Expand Up @@ -531,7 +531,7 @@ void TCPBase::Disconnect(const PeerAddress & address)

void TCPBase::OnPeerClosed(Inet::TCPEndPoint * endPoint)
{
TCPBase * tcp = reinterpret_cast<TCPBase *>(endPoint->AppState);
TCPBase * tcp = reinterpret_cast<TCPBase *>(endPoint->mAppState);

for (size_t i = 0; i < tcp->mActiveConnectionsSize; i++)
{
Expand Down
2 changes: 1 addition & 1 deletion src/transport/raw/UDP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<UDP *>(endPoint->AppState);
UDP * udp = reinterpret_cast<UDP *>(endPoint->mAppState);
PeerAddress peerAddress = PeerAddress::UDP(pktInfo->SrcAddress, pktInfo->SrcPort, pktInfo->Interface);

udp->HandleMessageReceived(peerAddress, std::move(buffer));
Expand Down

0 comments on commit 57ade85

Please sign in to comment.