Fix WakeEvent::Open() (#8551)
* Fix WakeEvent::Open()
#### Problem
Some tests were reporting,
```
CHIP:CSL: System wake event notify failed: OS Error 0x02000009: Bad file descriptor
```
but not failing.
#### Change overview
- Fix the problem (setting `mWriteFD` too late in `WakeEvent::Open()`).
- Add error returns and checks so that affected tests actually fail
without the above fix.
#### Testing
With the added error handling, but without the WakeEvent::Open() change,
numerous tests fail.
* review
* review
* fix missed #else case
diff --git a/src/inet/EndPointBasis.cpp b/src/inet/EndPointBasis.cpp
index 339051b..374057b 100644
--- a/src/inet/EndPointBasis.cpp
+++ b/src/inet/EndPointBasis.cpp
@@ -39,7 +39,7 @@
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
- mSocket.Init(aInetLayer.SystemLayer()->WatchableEvents());
+ (void) mSocket.Init(aInetLayer.SystemLayer()->WatchableEvents());
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
}
diff --git a/src/inet/IPEndPointBasis.cpp b/src/inet/IPEndPointBasis.cpp
index 5eb7d82..63c8c08 100644
--- a/src/inet/IPEndPointBasis.cpp
+++ b/src/inet/IPEndPointBasis.cpp
@@ -941,7 +941,7 @@
const int fd = ::socket(family, aType, aProtocol);
if (fd == -1)
return chip::System::MapErrorPOSIX(errno);
- mSocket.Attach(fd);
+ ReturnErrorOnFailure(mSocket.Attach(fd));
mAddrType = aAddressType;
diff --git a/src/inet/RawEndPoint.cpp b/src/inet/RawEndPoint.cpp
index c1df86b..c5580ee 100644
--- a/src/inet/RawEndPoint.cpp
+++ b/src/inet/RawEndPoint.cpp
@@ -425,7 +425,7 @@
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
// Wait for ability to read on this endpoint.
mSocket.SetCallback(HandlePendingIO, reinterpret_cast<intptr_t>(this));
- mSocket.RequestCallbackOnPendingRead();
+ ReturnErrorOnFailure(mSocket.RequestCallbackOnPendingRead());
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
return CHIP_NO_ERROR;
diff --git a/src/inet/TCPEndPoint.cpp b/src/inet/TCPEndPoint.cpp
index 1d58cab..661ab52 100644
--- a/src/inet/TCPEndPoint.cpp
+++ b/src/inet/TCPEndPoint.cpp
@@ -300,17 +300,19 @@
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
if (listen(mSocket.GetFD(), backlog) != 0)
+ {
res = chip::System::MapErrorPOSIX(errno);
+ }
else
{
// Enable non-blocking mode for the socket.
int flags = fcntl(mSocket.GetFD(), F_GETFL, 0);
fcntl(mSocket.GetFD(), F_SETFL, flags | O_NONBLOCK);
- }
- // Wait for ability to read on this endpoint.
- mSocket.SetCallback(HandlePendingIO, reinterpret_cast<intptr_t>(this));
- mSocket.RequestCallbackOnPendingRead();
+ // Wait for ability to read on this endpoint.
+ mSocket.SetCallback(HandlePendingIO, reinterpret_cast<intptr_t>(this));
+ res = mSocket.RequestCallbackOnPendingRead();
+ }
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
@@ -527,7 +529,7 @@
{
State = kState_Connected;
// Wait for ability to read on this endpoint.
- mSocket.RequestCallbackOnPendingRead();
+ ReturnErrorOnFailure(mSocket.RequestCallbackOnPendingRead());
if (OnConnectComplete != nullptr)
OnConnectComplete(this, CHIP_NO_ERROR);
}
@@ -535,7 +537,7 @@
{
State = kState_Connecting;
// Wait for ability to write on this endpoint.
- mSocket.RequestCallbackOnPendingWrite();
+ ReturnErrorOnFailure(mSocket.RequestCallbackOnPendingWrite());
}
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
@@ -800,7 +802,7 @@
mSendQueue = std::move(data);
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
// Wait for ability to write on this endpoint.
- mSocket.RequestCallbackOnPendingWrite();
+ ReturnErrorOnFailure(mSocket.RequestCallbackOnPendingWrite());
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
}
else
@@ -1423,7 +1425,11 @@
if (mSendQueue.IsNull())
{
// Do not wait for ability to write on this endpoint.
- mSocket.ClearCallbackOnPendingWrite();
+ err = mSocket.ClearCallbackOnPendingWrite();
+ if (err != CHIP_NO_ERROR)
+ {
+ break;
+ }
}
}
@@ -1526,8 +1532,16 @@
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
// Wait for ability to read or write on this endpoint.
- mSocket.RequestCallbackOnPendingRead();
- mSocket.RequestCallbackOnPendingWrite();
+ err = mSocket.RequestCallbackOnPendingRead();
+ if (err == CHIP_NO_ERROR)
+ {
+ err = mSocket.RequestCallbackOnPendingWrite();
+ }
+ if (err != CHIP_NO_ERROR)
+ {
+ DoClose(err, false);
+ return;
+ }
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
if (OnConnectComplete != nullptr)
@@ -1536,7 +1550,9 @@
// Otherwise, close the connection with an error.
else
+ {
DoClose(err, false);
+ }
}
CHIP_ERROR TCPEndPoint::DoClose(CHIP_ERROR err, bool suppressCallback)
@@ -1654,8 +1670,7 @@
ChipLogError(Inet, "SO_LINGER: %d", errno);
}
- if (mSocket.Close() != 0 && err == CHIP_NO_ERROR)
- err = chip::System::MapErrorPOSIX(errno);
+ mSocket.Close();
}
}
@@ -2420,7 +2435,7 @@
const int fd = ::socket(family, SOCK_STREAM | SOCK_FLAGS, 0);
if (fd == -1)
return chip::System::MapErrorPOSIX(errno);
- mSocket.Attach(fd);
+ ReturnErrorOnFailure(mSocket.Attach(fd));
mAddrType = addrType;
// If creating an IPv6 socket, tell the kernel that it will be IPv6 only. This makes it
@@ -2447,7 +2462,9 @@
#endif // defined(SO_NOSIGPIPE)
}
else if (mAddrType != addrType)
+ {
return CHIP_ERROR_INCORRECT_STATE;
+ }
return CHIP_NO_ERROR;
}
@@ -2616,7 +2633,7 @@
else
State = kState_Closing;
// Do not wait for ability to read on this endpoint.
- mSocket.ClearCallbackOnPendingRead();
+ (void) mSocket.ClearCallbackOnPendingRead();
// Call the app's OnPeerClose.
if (OnPeerClose != nullptr)
OnPeerClose(this);
@@ -2713,37 +2730,40 @@
if (err == CHIP_NO_ERROR)
{
// Put the new end point into the Connected state.
- conEP->mSocket.Attach(conSocket);
- conEP->State = kState_Connected;
+ err = conEP->mSocket.Attach(conSocket);
+ if (err == CHIP_NO_ERROR)
+ {
+ conEP->State = kState_Connected;
#if INET_CONFIG_ENABLE_IPV4
- conEP->mAddrType = (sa.any.sa_family == AF_INET6) ? kIPAddressType_IPv6 : kIPAddressType_IPv4;
+ conEP->mAddrType = (sa.any.sa_family == AF_INET6) ? kIPAddressType_IPv6 : kIPAddressType_IPv4;
#else // !INET_CONFIG_ENABLE_IPV4
- conEP->mAddrType = kIPAddressType_IPv6;
+ conEP->mAddrType = kIPAddressType_IPv6;
#endif // !INET_CONFIG_ENABLE_IPV4
- conEP->Retain();
+ conEP->Retain();
- // Wait for ability to read on this endpoint.
- conEP->mSocket.SetCallback(HandlePendingIO, reinterpret_cast<intptr_t>(conEP));
- conEP->mSocket.RequestCallbackOnPendingRead();
-
- // Call the app's callback function.
- OnConnectionReceived(this, conEP, peerAddr, peerPort);
+ // Wait for ability to read on this endpoint.
+ conEP->mSocket.SetCallback(HandlePendingIO, reinterpret_cast<intptr_t>(conEP));
+ err = conEP->mSocket.RequestCallbackOnPendingRead();
+ if (err == CHIP_NO_ERROR)
+ {
+ // Call the app's callback function.
+ OnConnectionReceived(this, conEP, peerAddr, peerPort);
+ return;
+ }
+ }
}
// Otherwise immediately close the connection, clean up and call the app's error callback.
- else
+ if (conSocket != -1)
+ close(conSocket);
+ if (conEP != nullptr)
{
- if (conSocket != -1)
- close(conSocket);
- if (conEP != nullptr)
- {
- if (conEP->State == kState_Connected)
- conEP->Release();
+ if (conEP->State == kState_Connected)
conEP->Release();
- }
- if (OnAcceptError != nullptr)
- OnAcceptError(this, err);
+ conEP->Release();
}
+ if (OnAcceptError != nullptr)
+ OnAcceptError(this, err);
}
#if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
diff --git a/src/inet/UDPEndPoint.cpp b/src/inet/UDPEndPoint.cpp
index df06a51..aebc428 100644
--- a/src/inet/UDPEndPoint.cpp
+++ b/src/inet/UDPEndPoint.cpp
@@ -351,7 +351,7 @@
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
// Wait for ability to read on this endpoint.
mSocket.SetCallback(HandlePendingIO, reinterpret_cast<intptr_t>(this));
- mSocket.RequestCallbackOnPendingRead();
+ ReturnErrorOnFailure(mSocket.RequestCallbackOnPendingRead());
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
return CHIP_NO_ERROR;
diff --git a/src/lib/support/CodeUtils.h b/src/lib/support/CodeUtils.h
index 0da7e47..d9afb28 100644
--- a/src/lib/support/CodeUtils.h
+++ b/src/lib/support/CodeUtils.h
@@ -532,6 +532,30 @@
#define VerifyOrDieWithMsg(aCondition, aModule, aMessage, ...) \
nlABORT_ACTION(aCondition, ChipLogDetail(aModule, aMessage, ##__VA_ARGS__))
+/**
+ * @def LogErrorOnFailure(expr)
+ *
+ * @brief
+ * Logs a message if the expression returns something different than CHIP_NO_ERROR.
+ *
+ * Example usage:
+ *
+ * @code
+ * ReturnLogErrorOnFailure(channel->SendMsg(msg));
+ * @endcode
+ *
+ * @param[in] expr A scalar expression to be evaluated against CHIP_NO_ERROR.
+ */
+#define LogErrorOnFailure(expr) \
+ do \
+ { \
+ CHIP_ERROR __err = (expr); \
+ if (__err != CHIP_NO_ERROR) \
+ { \
+ ChipLogError(NotSpecified, "%s at %s:%d", ErrorStr(__err), __FILE__, __LINE__); \
+ } \
+ } while (false)
+
#if (__cplusplus >= 201103L)
#ifndef __FINAL
diff --git a/src/platform/Linux/MdnsImpl.cpp b/src/platform/Linux/MdnsImpl.cpp
index 9a2f92f..042a7f5 100644
--- a/src/platform/Linux/MdnsImpl.cpp
+++ b/src/platform/Linux/MdnsImpl.cpp
@@ -163,11 +163,17 @@
VerifyOrDie(callback != nullptr && fd >= 0);
auto watch = std::make_unique<AvahiWatch>();
- watch->mSocket.Init(*mWatchableEvents)
- .Attach(fd)
- .SetCallback(AvahiWatchCallbackTrampoline, reinterpret_cast<intptr_t>(watch.get()))
- .RequestCallbackOnPendingRead(event & AVAHI_WATCH_IN)
- .RequestCallbackOnPendingWrite(event & AVAHI_WATCH_OUT);
+ watch->mSocket.Init(*mWatchableEvents);
+ LogErrorOnFailure(watch->mSocket.Attach(fd));
+ watch->mSocket.SetCallback(AvahiWatchCallbackTrampoline, reinterpret_cast<intptr_t>(watch.get()));
+ if (event & AVAHI_WATCH_IN)
+ {
+ LogErrorOnFailure(watch->mSocket.RequestCallbackOnPendingRead());
+ }
+ if (event & AVAHI_WATCH_OUT)
+ {
+ LogErrorOnFailure(watch->mSocket.RequestCallbackOnPendingWrite());
+ }
watch->mCallback = callback;
watch->mContext = context;
watch->mPoller = this;
@@ -180,19 +186,19 @@
{
if (event & AVAHI_WATCH_IN)
{
- watch->mSocket.RequestCallbackOnPendingRead();
+ LogErrorOnFailure(watch->mSocket.RequestCallbackOnPendingRead());
}
else
{
- watch->mSocket.ClearCallbackOnPendingRead();
+ LogErrorOnFailure(watch->mSocket.ClearCallbackOnPendingRead());
}
if (event & AVAHI_WATCH_OUT)
{
- watch->mSocket.RequestCallbackOnPendingWrite();
+ LogErrorOnFailure(watch->mSocket.RequestCallbackOnPendingWrite());
}
else
{
- watch->mSocket.ClearCallbackOnPendingWrite();
+ LogErrorOnFailure(watch->mSocket.ClearCallbackOnPendingWrite());
}
}
diff --git a/src/system/SystemLayer.cpp b/src/system/SystemLayer.cpp
index 6bf40d9..8c7ac6e 100644
--- a/src/system/SystemLayer.cpp
+++ b/src/system/SystemLayer.cpp
@@ -92,7 +92,7 @@
return CHIP_ERROR_INCORRECT_STATE;
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
- mWatchableEvents.Init(*this);
+ ReturnErrorOnFailure(mWatchableEvents.Init(*this));
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
#if CHIP_SYSTEM_CONFIG_USE_LWIP
this->AddEventHandlerDelegate(sSystemEventHandlerDelegate);
@@ -119,7 +119,7 @@
}
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
- mWatchableEvents.Shutdown();
+ ReturnErrorOnFailure(mWatchableEvents.Shutdown());
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
this->mLayerState = kLayerState_NotInitialized;
diff --git a/src/system/WakeEvent.cpp b/src/system/WakeEvent.cpp
index e0e387a..d9b9ca9 100644
--- a/src/system/WakeEvent.cpp
+++ b/src/system/WakeEvent.cpp
@@ -71,30 +71,21 @@
if (SetNonBlockingMode(fds[FD_WRITE]) < 0)
return chip::System::MapErrorPOSIX(errno);
- mFD.Init(watchState);
- mFD.Attach(fds[FD_READ]);
- mFD.SetCallback(Confirm, reinterpret_cast<intptr_t>(this));
- mFD.RequestCallbackOnPendingRead();
-
mWriteFD = fds[FD_WRITE];
+ mFD.Init(watchState);
+ ReturnErrorOnFailure(mFD.Attach(fds[FD_READ]));
+ mFD.SetCallback(Confirm, reinterpret_cast<intptr_t>(this));
+ ReturnErrorOnFailure(mFD.RequestCallbackOnPendingRead());
+
return CHIP_NO_ERROR;
}
-CHIP_ERROR WakeEvent::Close()
+void WakeEvent::Close()
{
- int res = 0;
-
- res |= mFD.Close();
- res |= ::close(mWriteFD);
+ mFD.Close();
+ VerifyOrDie(::close(mWriteFD) == 0);
mWriteFD = -1;
-
- if (res < 0)
- {
- return chip::System::MapErrorPOSIX(errno);
- }
-
- return CHIP_NO_ERROR;
}
void WakeEvent::Confirm()
@@ -144,16 +135,9 @@
return CHIP_NO_ERROR;
}
-CHIP_ERROR WakeEvent::Close()
+void WakeEvent::Close()
{
- int res = mFD.Close();
-
- if (res < 0)
- {
- return chip::System::MapErrorPOSIX(errno);
- }
-
- return CHIP_NO_ERROR;
+ mFD.Close();
}
void WakeEvent::Confirm()
diff --git a/src/system/WakeEvent.h b/src/system/WakeEvent.h
index 54465ed..727a64d 100644
--- a/src/system/WakeEvent.h
+++ b/src/system/WakeEvent.h
@@ -46,7 +46,7 @@
{
public:
CHIP_ERROR Open(WatchableEventManager & watchState); /**< Initialize the pipeline */
- CHIP_ERROR Close(); /**< Close both ends of the pipeline. */
+ void Close(); /**< Close both ends of the pipeline. */
CHIP_ERROR Notify(); /**< Set the event. */
void Confirm(); /**< Clear the event. */
diff --git a/src/system/WatchableEventManager.h b/src/system/WatchableEventManager.h
index bf22645..0ab91ce 100644
--- a/src/system/WatchableEventManager.h
+++ b/src/system/WatchableEventManager.h
@@ -41,9 +41,9 @@
*
* It MUST provide at least three methods:
*
- * - void Init(System::Layer & systemLayer) -- called from System::Layer::Init()
- * - void Shutdown() -- called from System::Layer::Shutdown()
- * - void Signal() -- called to indicate that event monitoring may need to be refreshed or resumed.
+ * - CHIP_ERROR Init(System::Layer & systemLayer) -- called from System::Layer::Init()
+ * - CHIP_ERROR Shutdown() -- called from System::Layer::Shutdown()
+ * - void Signal() -- called to indicate that event monitoring may need to be refreshed or resumed.
*
* Other contents depend on the contract between socket-event implementation and platform layer implementation.
* For POSIX-like platforms, WatchableEventManager provides a set of functions called from the event loop:
diff --git a/src/system/WatchableEventManagerLibevent.cpp b/src/system/WatchableEventManagerLibevent.cpp
index 8517b29..141b95f 100644
--- a/src/system/WatchableEventManagerLibevent.cpp
+++ b/src/system/WatchableEventManagerLibevent.cpp
@@ -61,7 +61,7 @@
} // anonymous namespace
-void WatchableEventManager::Init(System::Layer & systemLayer)
+CHIP_ERROR WatchableEventManager::Init(System::Layer & systemLayer)
{
#if CHIP_CONFIG_LIBEVENT_DEBUG_CHECKS
static bool enabled_event_debug_mode = false;
@@ -76,6 +76,7 @@
mTimeoutEvent = evtimer_new(mEventBase, TimeoutCallbackHandler, event_self_cbarg());
mActiveSockets = nullptr;
mSystemLayer = &systemLayer;
+ return CHIP_NO_ERROR;
}
void WatchableEventManager::PrepareEvents()
@@ -117,13 +118,14 @@
}
}
-void WatchableEventManager::Shutdown()
+CHIP_ERROR WatchableEventManager::Shutdown()
{
event_base_loopbreak(mEventBase);
event_free(mTimeoutEvent);
mTimeoutEvent = nullptr;
event_base_free(mEventBase);
mEventBase = nullptr;
+ return CHIP_NO_ERROR;
}
void WatchableEventManager::Signal()
diff --git a/src/system/WatchableEventManagerLibevent.h b/src/system/WatchableEventManagerLibevent.h
index 828b356..5bbab07 100644
--- a/src/system/WatchableEventManagerLibevent.h
+++ b/src/system/WatchableEventManagerLibevent.h
@@ -37,8 +37,8 @@
{
public:
WatchableEventManager() : mActiveSockets(nullptr), mSystemLayer(nullptr), mEventBase(nullptr), mTimeoutEvent(nullptr) {}
- void Init(Layer & systemLayer);
- void Shutdown();
+ CHIP_ERROR Init(Layer & systemLayer);
+ CHIP_ERROR Shutdown();
void Signal();
void EventLoopBegins() {}
diff --git a/src/system/WatchableEventManagerSelect.cpp b/src/system/WatchableEventManagerSelect.cpp
index f1f975b..a46fd95 100644
--- a/src/system/WatchableEventManagerSelect.cpp
+++ b/src/system/WatchableEventManagerSelect.cpp
@@ -45,7 +45,7 @@
namespace chip {
namespace System {
-void WatchableEventManager::Init(Layer & systemLayer)
+CHIP_ERROR WatchableEventManager::Init(Layer & systemLayer)
{
mSystemLayer = &systemLayer;
mMaxFd = -1;
@@ -54,13 +54,14 @@
FD_ZERO(&mRequest.mErrorSet);
// Create an event to allow an arbitrary thread to wake the thread in the select loop.
- mWakeEvent.Open(*this);
+ return mWakeEvent.Open(*this);
}
-void WatchableEventManager::Shutdown()
+CHIP_ERROR WatchableEventManager::Shutdown()
{
mWakeEvent.Close();
mSystemLayer = nullptr;
+ return CHIP_NO_ERROR;
}
void WatchableEventManager::Signal()
@@ -125,7 +126,7 @@
return FD_ISSET(fd, &mRequest.mReadSet) || FD_ISSET(fd, &mRequest.mWriteSet) || FD_ISSET(fd, &mRequest.mErrorSet);
}
-void WatchableEventManager::Set(int fd, fd_set * fds)
+CHIP_ERROR WatchableEventManager::Set(int fd, fd_set * fds)
{
FD_SET(fd, fds);
if (fd > mMaxFd)
@@ -134,9 +135,10 @@
}
// Wake the thread calling select so that it starts selecting on the new socket.
Signal();
+ return CHIP_NO_ERROR;
}
-void WatchableEventManager::Clear(int fd, fd_set * fds)
+CHIP_ERROR WatchableEventManager::Clear(int fd, fd_set * fds)
{
FD_CLR(fd, fds);
if (fd == mMaxFd)
@@ -145,6 +147,7 @@
}
// Wake the thread calling select so that it starts selecting on the new socket.
Signal();
+ return CHIP_NO_ERROR;
}
void WatchableEventManager::Reset(int fd)
diff --git a/src/system/WatchableEventManagerSelect.h b/src/system/WatchableEventManagerSelect.h
index a7bc285..2b12397 100644
--- a/src/system/WatchableEventManagerSelect.h
+++ b/src/system/WatchableEventManagerSelect.h
@@ -41,8 +41,8 @@
class WatchableEventManager
{
public:
- void Init(System::Layer & systemLayer);
- void Shutdown();
+ CHIP_ERROR Init(System::Layer & systemLayer);
+ CHIP_ERROR Shutdown();
void Signal();
void EventLoopBegins() {}
@@ -59,8 +59,8 @@
protected:
friend class WatchableSocket;
- void Set(int fd, fd_set * fds);
- void Clear(int fd, fd_set * fds);
+ CHIP_ERROR Set(int fd, fd_set * fds);
+ CHIP_ERROR Clear(int fd, fd_set * fds);
Layer * mSystemLayer = nullptr;
WatchableSocket * mAttachedSockets = nullptr;
diff --git a/src/system/WatchableSocket.h b/src/system/WatchableSocket.h
index 32ed69a..73615e0 100644
--- a/src/system/WatchableSocket.h
+++ b/src/system/WatchableSocket.h
@@ -27,9 +27,13 @@
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
+#include <errno.h>
#include <unistd.h>
+#include <core/CHIPError.h>
#include <support/BitFlags.h>
+#include <support/CodeUtils.h>
+#include <system/SystemError.h>
namespace chip {
@@ -57,12 +61,12 @@
* and provide the following methods, which are invoked by the corresponding WatchableSocketBasis functions:
*
* void OnInit()
- * void OnAttach()
- * void OnClose()
- * void OnRequestCallbackOnPendingRead()
- * void OnRequestCallbackOnPendingWrite()
- * void OnClearCallbackOnPendingRead()
- * void OnClearCallbackOnPendingWrite()
+ * CHIP_ERROR OnAttach()
+ * CHIP_ERROR OnRelease()
+ * CHIP_ERROR OnRequestCallbackOnPendingRead()
+ * CHIP_ERROR OnRequestCallbackOnPendingWrite()
+ * CHIP_ERROR OnClearCallbackOnPendingRead()
+ * CHIP_ERROR OnClearCallbackOnPendingWrite()
*
*/
class WatchableSocket;
@@ -96,16 +100,14 @@
*
* @param[in] manager Reference to shared socket-event state (which must already have been initialized).
*/
- Impl & Init(WatchableEventManager & manager)
+ void Init(WatchableEventManager & manager)
{
mFD = kInvalidFd;
mPendingIO.ClearAll();
- mCallback = nullptr;
- mCallbackData = 0;
- mSharedState = &manager;
- Impl * const impl = static_cast<Impl *>(this);
- impl->OnInit();
- return *impl;
+ mCallback = nullptr;
+ mCallbackData = 0;
+ mSharedState = &manager;
+ static_cast<Impl *>(this)->OnInit();
}
/**
@@ -113,12 +115,10 @@
*
* @param[in] fd An open file descriptor.
*/
- Impl & Attach(int fd)
+ CHIP_ERROR Attach(int fd)
{
- mFD = fd;
- Impl * const impl = static_cast<Impl *>(this);
- impl->OnAttach();
- return *impl;
+ mFD = fd;
+ return static_cast<Impl *>(this)->OnAttach();
}
/**
@@ -128,7 +128,7 @@
*/
int ReleaseFD()
{
- static_cast<Impl *>(this)->OnClose();
+ static_cast<Impl *>(this)->OnRelease();
const int fd = mFD;
mFD = kInvalidFd;
return fd;
@@ -136,10 +136,8 @@
/**
* Close the associated file descriptor.
- *
- * @returns the return value of `close(2)`.
*/
- int Close() { return close(ReleaseFD()); }
+ void Close() { VerifyOrDie(close(ReleaseFD()) == 0); }
/**
* Test whether there is an associated open file descriptor.
@@ -154,48 +152,25 @@
/**
* Indicate that the socket-event system should invoke the registered callback when the file descriptor is ready to read.
*/
- Impl & RequestCallbackOnPendingRead(bool request = true)
- {
- Impl * const impl = static_cast<Impl *>(this);
- if (request)
- {
- impl->OnRequestCallbackOnPendingRead();
- }
- return *impl;
- }
+ CHIP_ERROR RequestCallbackOnPendingRead() { return static_cast<Impl *>(this)->OnRequestCallbackOnPendingRead(); }
/**
* Indicate that the socket-event system should invoke the registered callback when the file descriptor is ready to write.
*/
- Impl & RequestCallbackOnPendingWrite(bool request = true)
+ CHIP_ERROR RequestCallbackOnPendingWrite(bool request = true)
{
- Impl * const impl = static_cast<Impl *>(this);
- if (request)
- {
- impl->OnRequestCallbackOnPendingWrite();
- }
- return *impl;
+ return static_cast<Impl *>(this)->OnRequestCallbackOnPendingWrite();
}
/**
* Indicate that the socket-event system need not invoke the registered callback when the file descriptor is ready to read.
*/
- Impl & ClearCallbackOnPendingRead()
- {
- Impl * const impl = static_cast<Impl *>(this);
- impl->OnClearCallbackOnPendingRead();
- return *impl;
- }
+ CHIP_ERROR ClearCallbackOnPendingRead() { return static_cast<Impl *>(this)->OnClearCallbackOnPendingRead(); }
/**
* Indicate that the socket-event system need not invoke the registered callback when the file descriptor is ready to write.
*/
- Impl & ClearCallbackOnPendingWrite()
- {
- Impl * const impl = static_cast<Impl *>(this);
- impl->OnClearCallbackOnPendingWrite();
- return *impl;
- }
+ CHIP_ERROR ClearCallbackOnPendingWrite() { return static_cast<Impl *>(this)->OnClearCallbackOnPendingWrite(); }
/**
* The callback is passed a reference to the WatchableSocket for which the requested event(s) are ready.
@@ -210,11 +185,10 @@
* @param[in] callback Function invoked when event(s) are ready.
* @param[in] data Arbitrary data accessible within a callback function.
*/
- Impl & SetCallback(Callback callback, intptr_t data)
+ void SetCallback(Callback callback, intptr_t data)
{
mCallback = callback;
mCallbackData = data;
- return *static_cast<Impl *>(this);
}
/**
diff --git a/src/system/WatchableSocketLibevent.cpp b/src/system/WatchableSocketLibevent.cpp
index 0913c97..f25213e 100644
--- a/src/system/WatchableSocketLibevent.cpp
+++ b/src/system/WatchableSocketLibevent.cpp
@@ -22,6 +22,7 @@
#include <platform/CHIPDeviceBuildConfig.h>
#include <support/CodeUtils.h>
+#include <system/SystemError.h>
#include <system/SystemLayer.h>
#include <system/WatchableEventManager.h>
#include <system/WatchableSocket.h>
@@ -35,38 +36,42 @@
mActiveNext = nullptr;
}
-void WatchableSocket::OnAttach()
+CHIP_ERROR WatchableSocket::OnAttach()
{
- evutil_make_socket_nonblocking(mFD);
+ VerifyOrReturnError(evutil_make_socket_nonblocking(mFD) == 0, MapErrorPOSIX(errno));
+ return CHIP_NO_ERROR;
}
-void WatchableSocket::OnClose()
+CHIP_ERROR WatchableSocket::OnRelease()
{
- UpdateWatch(0);
+ CHIP_ERROR status = UpdateWatch(0);
mSharedState->RemoveFromQueueIfPresent(this);
+ return status;
}
-void WatchableSocket::SetWatch(short eventFlags)
+CHIP_ERROR WatchableSocket::SetWatch(short eventFlags)
{
const short oldFlags = mEvent ? event_get_events(mEvent) : 0;
const short newFlags = static_cast<short>(EV_PERSIST | oldFlags | eventFlags);
if (oldFlags != newFlags)
{
- UpdateWatch(newFlags);
+ return UpdateWatch(newFlags);
}
+ return CHIP_NO_ERROR;
}
-void WatchableSocket::ClearWatch(short eventFlags)
+CHIP_ERROR WatchableSocket::ClearWatch(short eventFlags)
{
const short oldFlags = mEvent ? event_get_events(mEvent) : 0;
const short newFlags = static_cast<short>(EV_PERSIST | (oldFlags & ~eventFlags));
if (oldFlags != newFlags)
{
- UpdateWatch(newFlags);
+ return UpdateWatch(newFlags);
}
+ return CHIP_NO_ERROR;
}
-void WatchableSocket::UpdateWatch(short eventFlags)
+CHIP_ERROR WatchableSocket::UpdateWatch(short eventFlags)
{
if (mEvent)
{
@@ -78,8 +83,10 @@
{
event_base * const base = mSharedState->mEventBase;
mEvent = event_new(base, mFD, eventFlags, WatchableEventManager::LibeventCallbackHandler, this);
- event_add(mEvent, nullptr);
+ VerifyOrReturnError(mEvent != nullptr, CHIP_ERROR_NO_MEMORY);
+ VerifyOrReturnError(event_add(mEvent, nullptr) == 0, CHIP_ERROR_INTERNAL);
}
+ return CHIP_NO_ERROR;
}
} // namespace System
diff --git a/src/system/WatchableSocketLibevent.h b/src/system/WatchableSocketLibevent.h
index be2448a..8959210 100644
--- a/src/system/WatchableSocketLibevent.h
+++ b/src/system/WatchableSocketLibevent.h
@@ -39,19 +39,19 @@
{
public:
void OnInit();
- void OnAttach();
- void OnClose();
- void OnRequestCallbackOnPendingRead() { SetWatch(EV_READ); }
- void OnRequestCallbackOnPendingWrite() { SetWatch(EV_WRITE); }
- void OnClearCallbackOnPendingRead() { ClearWatch(EV_READ); }
- void OnClearCallbackOnPendingWrite() { ClearWatch(EV_WRITE); }
+ CHIP_ERROR OnAttach();
+ CHIP_ERROR OnRelease();
+ CHIP_ERROR OnRequestCallbackOnPendingRead() { return SetWatch(EV_READ); }
+ CHIP_ERROR OnRequestCallbackOnPendingWrite() { return SetWatch(EV_WRITE); }
+ CHIP_ERROR OnClearCallbackOnPendingRead() { return ClearWatch(EV_READ); }
+ CHIP_ERROR OnClearCallbackOnPendingWrite() { return ClearWatch(EV_WRITE); }
private:
friend class WatchableEventManager;
- void SetWatch(short eventFlags);
- void ClearWatch(short eventFlags);
- void UpdateWatch(short eventFlags);
+ CHIP_ERROR SetWatch(short eventFlags);
+ CHIP_ERROR ClearWatch(short eventFlags);
+ CHIP_ERROR UpdateWatch(short eventFlags);
WatchableSocket * mActiveNext; ///< Next element in the list of sockets activated by libevent.
struct event * mEvent; ///< libevent state.
diff --git a/src/system/WatchableSocketSelect.cpp b/src/system/WatchableSocketSelect.cpp
index 04f8dbb..e418493 100644
--- a/src/system/WatchableSocketSelect.cpp
+++ b/src/system/WatchableSocketSelect.cpp
@@ -32,18 +32,19 @@
namespace chip {
namespace System {
-void WatchableSocket::OnAttach()
+CHIP_ERROR WatchableSocket::OnAttach()
{
mSharedState->Reset(mFD);
- VerifyOrDie(mAttachedNext == nullptr);
+ VerifyOrReturnError(mAttachedNext == nullptr, CHIP_ERROR_INCORRECT_STATE);
mAttachedNext = mSharedState->mAttachedSockets;
mSharedState->mAttachedSockets = this;
+ return CHIP_NO_ERROR;
}
-void WatchableSocket::OnClose()
+CHIP_ERROR WatchableSocket::OnRelease()
{
- VerifyOrDie(mFD >= 0);
+ VerifyOrReturnError(mFD >= 0, CHIP_ERROR_INCORRECT_STATE);
mSharedState->Reset(mFD);
WatchableSocket ** pp = &mSharedState->mAttachedSockets;
@@ -57,30 +58,29 @@
pp = &(*pp)->mAttachedNext;
}
-#if CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK
// Wake the thread calling select so that it stops selecting on the socket.
mSharedState->Signal();
-#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK
+ return CHIP_NO_ERROR;
}
-void WatchableSocket::OnRequestCallbackOnPendingRead()
+CHIP_ERROR WatchableSocket::OnRequestCallbackOnPendingRead()
{
- mSharedState->Set(mFD, &mSharedState->mRequest.mReadSet);
+ return mSharedState->Set(mFD, &mSharedState->mRequest.mReadSet);
}
-void WatchableSocket::OnRequestCallbackOnPendingWrite()
+CHIP_ERROR WatchableSocket::OnRequestCallbackOnPendingWrite()
{
- mSharedState->Set(mFD, &mSharedState->mRequest.mWriteSet);
+ return mSharedState->Set(mFD, &mSharedState->mRequest.mWriteSet);
}
-void WatchableSocket::OnClearCallbackOnPendingRead()
+CHIP_ERROR WatchableSocket::OnClearCallbackOnPendingRead()
{
- mSharedState->Clear(mFD, &mSharedState->mRequest.mReadSet);
+ return mSharedState->Clear(mFD, &mSharedState->mRequest.mReadSet);
}
-void WatchableSocket::OnClearCallbackOnPendingWrite()
+CHIP_ERROR WatchableSocket::OnClearCallbackOnPendingWrite()
{
- mSharedState->Clear(mFD, &mSharedState->mRequest.mWriteSet);
+ return mSharedState->Clear(mFD, &mSharedState->mRequest.mWriteSet);
}
/**
diff --git a/src/system/WatchableSocketSelect.h b/src/system/WatchableSocketSelect.h
index 266ad7e..2d32f98 100644
--- a/src/system/WatchableSocketSelect.h
+++ b/src/system/WatchableSocketSelect.h
@@ -39,13 +39,13 @@
{
public:
void OnInit() { mAttachedNext = nullptr; }
- void OnAttach();
- void OnClose();
+ CHIP_ERROR OnAttach();
+ CHIP_ERROR OnRelease();
- void OnRequestCallbackOnPendingRead();
- void OnRequestCallbackOnPendingWrite();
- void OnClearCallbackOnPendingRead();
- void OnClearCallbackOnPendingWrite();
+ CHIP_ERROR OnRequestCallbackOnPendingRead();
+ CHIP_ERROR OnRequestCallbackOnPendingWrite();
+ CHIP_ERROR OnClearCallbackOnPendingRead();
+ CHIP_ERROR OnClearCallbackOnPendingWrite();
void SetPendingIO(SocketEvents events) { mPendingIO = events; }
void SetFDs(int & nfds, fd_set * readfds, fd_set * writefds, fd_set * exceptfds);
diff --git a/src/system/tests/TestSystemWakeEvent.cpp b/src/system/tests/TestSystemWakeEvent.cpp
index 4dbfd0c..54d2eb5 100644
--- a/src/system/tests/TestSystemWakeEvent.cpp
+++ b/src/system/tests/TestSystemWakeEvent.cpp
@@ -66,7 +66,7 @@
TestContext()
{
- mWatchableEvents.Init(mSystemLayer);
+ (void) mWatchableEvents.Init(mSystemLayer);
mWakeEvent.Open(mWatchableEvents);
}
~TestContext() { mWakeEvent.Close(); }
