Skip to content

Commit

Permalink
[Inet] Miscellaneous cleanup (#12968)
Browse files Browse the repository at this point in the history
#### Change overview

- Remove the pointer `union` from `EndPointStateLwIP`; the members can be safe private members of the respective EndPoint classes.
- Remove some unused InetConfig definitions.
- Remove the unused `IPProtocol` enum.
- Streamline `EndPointManagerImplPool` size. Add `k` to constants.
- Additional init/shutdown order checks; fixed `TestAdvertiser` and `messaging/tests/echo` shutdown.

#### Testing

CI; no additional functionality.
  • Loading branch information
kpschoedel authored and pull[bot] committed Jan 4, 2022
1 parent 875ccc5 commit 1111428
Show file tree
Hide file tree
Showing 17 changed files with 36 additions and 101 deletions.
15 changes: 3 additions & 12 deletions src/inet/EndPointStateLwIP.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,14 @@ struct tcp_pcb;
namespace chip {
namespace Inet {

/**
* Definitions shared by all LwIP EndPoint classes.
*/
class DLL_EXPORT EndPointStateLwIP
{
protected:
EndPointStateLwIP() : mLwIPEndPointType(LwIPEndPointType::Unknown) {}

/** Encapsulated LwIP protocol control block */
union
{
const void * mVoid; /**< An untyped protocol control buffer reference */
#if INET_CONFIG_ENABLE_UDP_ENDPOINT
udp_pcb * mUDP; /**< User datagram protocol (UDP) control */
#endif // INET_CONFIG_ENABLE_UDP_ENDPOINT
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
tcp_pcb * mTCP; /**< Transmission control protocol (TCP) control */
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
};

enum class LwIPEndPointType : uint8_t
{
Unknown = 0,
Expand Down
3 changes: 3 additions & 0 deletions src/inet/EndPointStateSockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
namespace chip {
namespace Inet {

/**
* Definitions shared by all sockets-based EndPoint classes.
*/
class DLL_EXPORT EndPointStateSockets
{
protected:
Expand Down
14 changes: 0 additions & 14 deletions src/inet/IANAConstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,6 @@ typedef enum
#endif // INET_CONFIG_ENABLE_IPV4
} IPVersion;

/**
* @enum IPProtocol
*
* The numbers of some of the protocols in the IP family.
*
*/
typedef enum
{
kIPProtocol_ICMPv6 = 58, /**< ICMPv6 */
#if INET_CONFIG_ENABLE_IPV4
kIPProtocol_ICMPv4 = 1, /**< ICMPv4 */
#endif // INET_CONFIG_ENABLE_IPV4
} IPProtocol;

/**
* @brief Internet protocol multicast address scope
*
Expand Down
52 changes: 0 additions & 52 deletions src/inet/InetConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,32 +87,6 @@
#define INET_CONFIG_MAX_IP_AND_UDP_HEADER_SIZE (40 + 8)
#endif // INET_CONFIG_MAX_IP_AND_UDP_HEADER_SIZE

/**
* @def INET_CONFIG_WILL_OVERRIDE_OS_ERROR_FUNCS
*
* @brief
* This defines whether (1) or not (0) your platform will override
* the platform- and system-specific INET_MapOSError,
* INET_DescribeOSError, and INET_IsOSError functions.
*
*/
#ifndef INET_CONFIG_WILL_OVERRIDE_OS_ERROR_FUNCS
#define INET_CONFIG_WILL_OVERRIDE_OS_ERROR_FUNCS 0
#endif // INET_CONFIG_WILL_OVERRIDE_OS_ERROR_FUNCS

/**
* @def INET_CONFIG_WILL_OVERRIDE_LWIP_ERROR_FUNCS
*
* @brief
* This defines whether (1) or not (0) your platform will override
* the platform- and system-specific INET_MapLwIPError,
* INET_DescribeLwIPError, and INET_IsLwIPError functions.
*
*/
#ifndef INET_CONFIG_WILL_OVERRIDE_LWIP_ERROR_FUNCS
#define INET_CONFIG_WILL_OVERRIDE_LWIP_ERROR_FUNCS 0
#endif // INET_CONFIG_WILL_OVERRIDE_LWIP_ERROR_FUNCS

/**
* @def INET_CONFIG_NUM_TCP_ENDPOINTS
*
Expand Down Expand Up @@ -177,32 +151,6 @@
#define INET_CONFIG_ENABLE_UDP_ENDPOINT 0
#endif // INET_CONFIG_ENABLE_UDP_ENDPOINT

/**
* @def INET_CONFIG_EVENT_RESERVED
*
* @brief
* This defines the first number in the default chip System Layer event code space reserved for use by the Inet Layer.
* Event codes used by each layer must not overlap.
*/
#ifndef INET_CONFIG_EVENT_RESERVED
#define INET_CONFIG_EVENT_RESERVED 1000
#endif /* INET_CONFIG_EVENT_RESERVED */

/**
* @def _INET_CONFIG_EVENT
*
* @brief
* This defines a mapping function for InetLayer event types that allows
* mapping such event types into a platform- or system-specific range.
*
* @note
* By default, this definition is a copy of _CHIP_SYSTEM_CONFIG_LWIP_EVENT.
*
*/
#ifndef _INET_CONFIG_EVENT
#define _INET_CONFIG_EVENT(e) _CHIP_SYSTEM_CONFIG_LWIP_EVENT(INET_CONFIG_EVENT_RESERVED + (e))
#endif // _INET_CONFIG_EVENT

/**
* @def INET_CONFIG_TEST
*
Expand Down
3 changes: 0 additions & 3 deletions src/inet/InetError.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@
* Error types, ranges, and mappings overrides may be made by
* defining the appropriate INET_CONFIG_* or _INET_CONFIG_*
* macros.
*
* NOTE WELL: On some platforms, this header is included by C-language programs.
*
*/

#pragma once
Expand Down
18 changes: 10 additions & 8 deletions src/inet/InetLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ namespace Inet {
* Template providing traits for EndPoint types used by EndPointManager.
*
* Instances must define:
* static constexpr const char * Name;
* static constexpr int SystemStatsKey;
* static constexpr const char * kName;
* static constexpr int kSystemStatsKey;
*/
template <class EndPointType>
struct EndPointProperties;
Expand All @@ -56,12 +56,13 @@ class EndPointManager
using EndPointVisitor = Loop (*)(EndPoint *);

EndPointManager() {}
virtual ~EndPointManager() {}
virtual ~EndPointManager() { VerifyOrDie(mLayerState.Destroy()); }

CHIP_ERROR Init(System::Layer & systemLayer)
{
RegisterLayerErrorFormatter();
VerifyOrReturnError(mLayerState.SetInitializing(), CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(systemLayer.IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
mSystemLayer = &systemLayer;
mLayerState.SetInitialized();
return CHIP_NO_ERROR;
Expand All @@ -71,6 +72,7 @@ class EndPointManager
{
// Return to uninitialized state to permit re-initialization.
VerifyOrReturnError(mLayerState.ResetFromInitialized(), CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mSystemLayer->IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
mSystemLayer = nullptr;
return CHIP_NO_ERROR;
}
Expand All @@ -85,17 +87,17 @@ class EndPointManager
*retEndPoint = CreateEndPoint();
if (*retEndPoint == nullptr)
{
ChipLogError(Inet, "%s endpoint pool FULL", EndPointProperties<EndPointType>::Name);
ChipLogError(Inet, "%s endpoint pool FULL", EndPointProperties<EndPointType>::kName);
return CHIP_ERROR_ENDPOINT_POOL_FULL;
}

SYSTEM_STATS_INCREMENT(EndPointProperties<EndPointType>::SystemStatsKey);
SYSTEM_STATS_INCREMENT(EndPointProperties<EndPointType>::kSystemStatsKey);
return CHIP_NO_ERROR;
}

void DeleteEndPoint(EndPoint * endPoint)
{
SYSTEM_STATS_DECREMENT(EndPointProperties<EndPointType>::SystemStatsKey);
SYSTEM_STATS_DECREMENT(EndPointProperties<EndPointType>::kSystemStatsKey);
ReleaseEndPoint(endPoint);
}

Expand All @@ -108,7 +110,7 @@ class EndPointManager
System::Layer * mSystemLayer;
};

template <typename EndPointImpl, unsigned int NUM_ENDPOINTS>
template <typename EndPointImpl>
class EndPointManagerImplPool : public EndPointManager<typename EndPointImpl::EndPoint>
{
public:
Expand All @@ -126,7 +128,7 @@ class EndPointManagerImplPool : public EndPointManager<typename EndPointImpl::En
}

private:
ObjectPool<EndPointImpl, NUM_ENDPOINTS> sEndPointPool;
ObjectPool<EndPointImpl, EndPointProperties<EndPoint>::kNumEndPoints> sEndPointPool;
};

class TCPEndPoint;
Expand Down
5 changes: 3 additions & 2 deletions src/inet/TCPEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -691,8 +691,9 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
template <>
struct EndPointProperties<TCPEndPoint>
{
static constexpr const char * Name = "TCP";
static constexpr int SystemStatsKey = System::Stats::kInetLayer_NumTCPEps;
static constexpr const char * kName = "TCP";
static constexpr size_t kNumEndPoints = INET_CONFIG_NUM_TCP_ENDPOINTS;
static constexpr int kSystemStatsKey = System::Stats::kInetLayer_NumTCPEps;
};

} // namespace Inet
Expand Down
2 changes: 1 addition & 1 deletion src/inet/TCPEndPointImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
namespace chip {
namespace Inet {

using TCPEndPointManagerImpl = EndPointManagerImplPool<TCPEndPointImpl, INET_CONFIG_NUM_TCP_ENDPOINTS>;
using TCPEndPointManagerImpl = EndPointManagerImplPool<TCPEndPointImpl>;

} // namespace Inet
} // namespace chip
5 changes: 4 additions & 1 deletion src/inet/TCPEndPointImplLwIP.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ namespace Inet {
class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP
{
public:
TCPEndPointImplLwIP(EndPointManager<TCPEndPoint> & endPointManager) : TCPEndPoint(endPointManager), mUnackedLength(0) {}
TCPEndPointImplLwIP(EndPointManager<TCPEndPoint> & endPointManager) :
TCPEndPoint(endPointManager), mUnackedLength(0), mTCP(nullptr)
{}

// TCPEndPoint overrides.
CHIP_ERROR GetPeerInfo(IPAddress * retAddr, uint16_t * retPort) const override;
Expand Down Expand Up @@ -79,6 +81,7 @@ class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP

uint16_t mUnackedLength; // Amount sent but awaiting ACK. Used as a form of reference count
// to hang-on to backing packet buffers until they are no longer needed.
tcp_pcb * mTCP; // LwIP Transmission control protocol (TCP) control block.

uint16_t RemainingToSend();
BufferOffset FindStartOfUnsent();
Expand Down
5 changes: 3 additions & 2 deletions src/inet/UDPEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@ class DLL_EXPORT UDPEndPoint : public EndPointBasis<UDPEndPoint>
template <>
struct EndPointProperties<UDPEndPoint>
{
static constexpr const char * Name = "UDP";
static constexpr int SystemStatsKey = System::Stats::kInetLayer_NumUDPEps;
static constexpr const char * kName = "UDP";
static constexpr size_t kNumEndPoints = INET_CONFIG_NUM_UDP_ENDPOINTS;
static constexpr int kSystemStatsKey = System::Stats::kInetLayer_NumUDPEps;
};

} // namespace Inet
Expand Down
2 changes: 1 addition & 1 deletion src/inet/UDPEndPointImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
namespace chip {
namespace Inet {

using UDPEndPointManagerImpl = EndPointManagerImplPool<UDPEndPointImpl, INET_CONFIG_NUM_UDP_ENDPOINTS>;
using UDPEndPointManagerImpl = EndPointManagerImplPool<UDPEndPointImpl>;

} // namespace Inet
} // namespace chip
4 changes: 3 additions & 1 deletion src/inet/UDPEndPointImplLwIP.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace Inet {
class UDPEndPointImplLwIP : public UDPEndPoint, public EndPointStateLwIP
{
public:
UDPEndPointImplLwIP(EndPointManager<UDPEndPoint> & endPointManager) : UDPEndPoint(endPointManager) {}
UDPEndPointImplLwIP(EndPointManager<UDPEndPoint> & endPointManager) : UDPEndPoint(endPointManager), mUDP(nullptr) {}

// UDPEndPoint overrides.
CHIP_ERROR SetMulticastLoopback(IPVersion aIPVersion, bool aLoopback) override;
Expand Down Expand Up @@ -84,6 +84,8 @@ class UDPEndPointImplLwIP : public UDPEndPoint, public EndPointStateLwIP
#else // LWIP_VERSION_MAJOR <= 1 && LWIP_VERSION_MINOR < 5
static void LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb, struct pbuf * p, ip_addr_t * addr, u16_t port);
#endif // LWIP_VERSION_MAJOR > 1 || LWIP_VERSION_MINOR >= 5

udp_pcb * mUDP; // LwIP User datagram protocol (UDP) control block.
};

using UDPEndPointImpl = UDPEndPointImplLwIP;
Expand Down
1 change: 1 addition & 0 deletions src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ int TestAdvertiser(void)
mdnsAdvertiser.Init(context.GetUDPEndPointManager());
nlTestRunner(&theSuite, &server);
server.Shutdown();
context.Shutdown();
return nlTestRunnerStats(&theSuite);
}

Expand Down
1 change: 1 addition & 0 deletions src/messaging/tests/echo/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,6 @@ void ShutdownChip(void)
gMessageCounterManager.Shutdown();
gExchangeManager.Shutdown();
gSessionManager.Shutdown();
(void) chip::DeviceLayer::TCPEndPointManager()->Shutdown();
chip::DeviceLayer::PlatformMgr().Shutdown();
}
1 change: 0 additions & 1 deletion src/system/SystemLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include <lib/support/CodeUtils.h>
#include <lib/support/DLLUtil.h>
#include <lib/support/LambdaBridge.h>
#include <lib/support/ObjectLifeCycle.h>
#include <system/SystemClock.h>
#include <system/SystemError.h>
#include <system/SystemEvent.h>
Expand Down
2 changes: 1 addition & 1 deletion src/system/SystemLayerImplLwIP.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class LayerImplLwIP : public LayerLwIP
{
public:
LayerImplLwIP();
~LayerImplLwIP() = default;
~LayerImplLwIP() { VerifyOrDie(mLayerState.Destroy()); }

// Layer overrides.
CHIP_ERROR Init() override;
Expand Down
4 changes: 2 additions & 2 deletions src/system/SystemLayerImplSelect.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ namespace System {
class LayerImplSelect : public LayerSocketsLoop
{
public:
LayerImplSelect() = default;
~LayerImplSelect() = default;
LayerImplSelect() = default;
~LayerImplSelect() { VerifyOrDie(mLayerState.Destroy()); }

// Layer overrides.
CHIP_ERROR Init() override;
Expand Down

0 comments on commit 1111428

Please sign in to comment.