From b5dc794202b2027824f464d1b36ccc8becb94548 Mon Sep 17 00:00:00 2001 From: Patrik Wenger Date: Tue, 12 Apr 2016 20:10:54 +0200 Subject: [PATCH 1/2] Problem: zmq_poller_wait doesn't check *event arg Solution: use zmq_assert to ensure it's not a nullpointer --- src/zmq.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/zmq.cpp b/src/zmq.cpp index a5577108..e74f4f87 100644 --- a/src/zmq.cpp +++ b/src/zmq.cpp @@ -1209,6 +1209,8 @@ int zmq_poller_wait (void *poller_, zmq_poller_event_t *event, long timeout_) return -1; } + zmq_assert (event != NULL); + zmq::socket_poller_t::event_t e; memset (&e, 0, sizeof (e)); From 621c965fae5869732a30a203e6e2bb7c6e647a14 Mon Sep 17 00:00:00 2001 From: Patrik Wenger Date: Tue, 12 Apr 2016 20:11:50 +0200 Subject: [PATCH 2/2] Problem: tricky return value from zmq::socket_poller_t::wait when poller is empty Solution: return -1 (no event) instead of 0 (event) For some reason, this just returns 0 if there are no sockets registered on the poller. Usually this would mean there has been an event. So the caller would have to check the return value AND the event, or write code that takes the number of registered sockets into consideration. By returning -1 and setting errno = ETIMEDOUT like in the usual timeout cases, it's more consistent and convenient. Test case included. --- src/socket_poller.cpp | 14 ++++++++++---- tests/test_poller.cpp | 9 ++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/socket_poller.cpp b/src/socket_poller.cpp index 3a567173..d4345853 100644 --- a/src/socket_poller.cpp +++ b/src/socket_poller.cpp @@ -393,16 +393,22 @@ int zmq::socket_poller_t::wait (zmq::socket_poller_t::event_t *event_, long time #if defined ZMQ_POLL_BASED_ON_POLL if (unlikely (poll_size == 0)) { + // We'll report an error (timed out) as if the list was non-empty and + // no event occured within the specified timeout. Otherwise the caller + // needs to check the return value AND the event to avoid using the + // nullified event data. + errno = ETIMEDOUT; if (timeout_ == 0) - return 0; + return -1; #if defined ZMQ_HAVE_WINDOWS Sleep (timeout_ > 0 ? timeout_ : INFINITE); - return 0; + return -1; #elif defined ZMQ_HAVE_ANDROID usleep (timeout_ * 1000); - return 0; + return -1; #else - return usleep (timeout_ * 1000); + usleep (timeout_ * 1000); + return -1; #endif } diff --git a/tests/test_poller.cpp b/tests/test_poller.cpp index b8f26815..81824105 100644 --- a/tests/test_poller.cpp +++ b/tests/test_poller.cpp @@ -60,6 +60,14 @@ int main (void) // Set up poller void* poller = zmq_poller_new (); + zmq_poller_event_t event; + + // waiting on poller with no registered sockets should report error + rc = zmq_poller_wait(poller, &event, 0); + assert (rc == -1); + assert (errno == ETIMEDOUT); + + // register sink rc = zmq_poller_add (poller, sink, sink, ZMQ_POLLIN); assert (rc == 0); @@ -69,7 +77,6 @@ int main (void) assert (rc == 1); // We expect a message only on the sink - zmq_poller_event_t event; rc = zmq_poller_wait (poller, &event, -1); assert (rc == 0); assert (event.socket == sink);