changeset 590:3058eaa603e6

Net: improve Listener::unset and its tests
author David Demelier <markand@malikania.fr>
date Tue, 30 Aug 2016 13:03:38 +0200
parents 5717cb946cd3
children 3547fc5afba2
files modules/net/net.hpp modules/net/test/main.cpp
diffstat 2 files changed, 55 insertions(+), 74 deletions(-) [+]
line wrap: on
line diff
--- a/modules/net/net.hpp	Tue Aug 30 12:48:10 2016 +0200
+++ b/modules/net/net.hpp	Tue Aug 30 13:03:38 2016 +0200
@@ -3515,7 +3515,6 @@
      */
     void reset(Handle sc, Condition condition)
     {
-        // Invalid or useless flags.
         if (condition == Condition::None || static_cast<int>(condition) > 0x3)
             return;
 
@@ -3552,11 +3551,12 @@
     }
 
     /**
-     * Unset a socket from the listener, only the flags is removed
-     * unless the two flagss are requested.
+     * Remove one or more flags from the listener for the given socket.
      *
-     * For example, if you added a socket for both reading and writing,
-     * unsetting the write flags will keep the socket for reading.
+     * It's perfectly safe to try removing a flag that is not currently set, the
+     * listener simply ignores them.
+     *
+     * If the socket is not in the registry, this function is a no-op.
      *
      * \param sc the socket
      * \param condition the condition (may be OR'ed)
@@ -3564,31 +3564,32 @@
      */
     void unset(Handle sc, Condition condition)
     {
+        if (condition == Condition::None || static_cast<int>(condition) > 0x3)
+            return;
+
         auto it = m_table.find(sc);
 
-        // Invalid or useless flags.
-        if (condition == Condition::None || static_cast<int>(condition) > 0x3 || it == m_table.end())
+        if (it == m_table.end())
+            return;
+
+        // Remove only present flags.
+        condition = it->second & condition;
+
+        if (condition == Condition::None)
             return;
 
-        // Like set, do not update if the socket is already at the appropriate state.
-        if ((condition & Condition::Readable) == Condition::Readable &&
-            (it->second & Condition::Readable) != Condition::Readable)
-            condition &= ~(Condition::Readable);
-        if ((condition & Condition::Writable) == Condition::Writable &&
-            (it->second & Condition::Writable) != Condition::Writable)
-            condition &= ~(Condition::Writable);
-
-        if (condition != Condition::None) {
-            // Determine if it's a complete removal.
-            bool removal = ((it->second) & ~(condition)) == Condition::None;
-
-            m_backend.unset(m_table, sc, condition, removal);
-
-            if (removal)
-                m_table.erase(it);
-            else
-                it->second &= ~(condition);
-        }
+        /*
+         * Determine if we should completely remove the socket from the table if
+         * condition once remove is none.
+         */
+        bool removal = (it->second & ~(condition)) == Condition::None;
+
+        m_backend.unset(m_table, sc, condition, removal);
+
+        if (removal)
+            m_table.erase(it);
+        else
+            it->second &= ~(condition);
     }
 
     /**
--- a/modules/net/test/main.cpp	Tue Aug 30 12:48:10 2016 +0200
+++ b/modules/net/test/main.cpp	Tue Aug 30 13:03:38 2016 +0200
@@ -253,12 +253,14 @@
 
     inline void set(const ListenerTable &, Handle, Condition flags, bool addition) noexcept
     {
-        onSet(flags, addition);
+        if (onSet)
+            onSet(flags, addition);
     }
 
     inline void unset(const ListenerTable &, Handle, Condition flags, bool removal) noexcept
     {
-        onUnset(flags, removal);
+        if (onUnset)
+            onUnset(flags, removal);
     }
 
     std::vector<ListenerStatus> wait(const ListenerTable &, int)
@@ -480,32 +482,6 @@
  * ------------------------------------------------------------------
  */
 
-class TestBackendUnset {
-public:
-    bool m_isset{false};
-    bool m_isunset{false};
-    Condition m_flags{Condition::None};
-    bool m_removal{false};
-
-    inline void set(const ListenerTable &, Handle &, Condition flags, bool) noexcept
-    {
-        m_isset = true;
-        m_flags |= flags;
-    }
-
-    inline void unset(const ListenerTable &, Handle &, Condition flags, bool remove) noexcept
-    {
-        m_isunset = true;
-        m_flags &= ~(flags);
-        m_removal = remove;
-    }
-
-    std::vector<ListenerStatus> wait(const ListenerTable &, int) noexcept
-    {
-        return {};
-    }
-};
-
 class TestBackendUnsetFail {
 public:
     inline void set(const ListenerTable &, Handle &, Condition, bool) noexcept
@@ -525,63 +501,67 @@
 
 TEST(ListenerUnsetRemove, unset)
 {
-    Listener<TestBackendUnset> listener;
+    Listener<TestBackendSet> listener;
     Handle s = 0;
 
     listener.reset(s, Condition::Readable);
+    listener.backend().onUnset = [&] (auto flags, auto removal) {
+        ASSERT_EQ(Condition::Readable, flags);
+        ASSERT_TRUE(removal);
+    };
     listener.unset(s, Condition::Readable);
 
     ASSERT_EQ(0U, listener.size());
-    ASSERT_TRUE(listener.backend().m_isset);
-    ASSERT_TRUE(listener.backend().m_isunset);
-    ASSERT_TRUE(listener.backend().m_flags == Condition::None);
-    ASSERT_TRUE(listener.backend().m_removal);
 }
 
 TEST(ListenerUnsetRemove, unsetOne)
 {
-    Listener<TestBackendUnset> listener;
+    Listener<TestBackendSet> listener;
     Handle s = 0;
 
     listener.reset(s, Condition::Readable | Condition::Writable);
+    listener.backend().onUnset = [&] (auto flags, auto removal) {
+        ASSERT_EQ(Condition::Readable, flags);
+        ASSERT_FALSE(removal);
+    };
     listener.unset(s, Condition::Readable);
 
     ASSERT_EQ(1U, listener.size());
-    ASSERT_TRUE(listener.backend().m_isset);
-    ASSERT_TRUE(listener.backend().m_isunset);
-    ASSERT_TRUE(listener.backend().m_flags == Condition::Writable);
-    ASSERT_FALSE(listener.backend().m_removal);
 }
 
 TEST(ListenerUnsetRemove, unsetAll)
 {
-    Listener<TestBackendUnset> listener;
+    Listener<TestBackendSet> listener;
     Handle s = 0;
 
     listener.reset(s, Condition::Readable | Condition::Writable);
+    listener.backend().onUnset = [&] (auto flags, auto removal) {
+        ASSERT_EQ(Condition::Readable, flags);
+        ASSERT_FALSE(removal);
+    };
     listener.unset(s, Condition::Readable);
+    listener.backend().onUnset = [&] (auto flags, auto removal) {
+        ASSERT_EQ(Condition::Writable, flags);
+        ASSERT_TRUE(removal);
+    };
     listener.unset(s, Condition::Writable);
 
     ASSERT_EQ(0U, listener.size());
-    ASSERT_TRUE(listener.backend().m_isset);
-    ASSERT_TRUE(listener.backend().m_isunset);
-    ASSERT_TRUE(listener.backend().m_flags == Condition::None);
-    ASSERT_TRUE(listener.backend().m_removal);
 }
 
 TEST(ListenerUnsetRemove, remove)
 {
-    Listener<TestBackendUnset> listener;
+    Listener<TestBackendSet> listener;
     Handle s = 0;
 
     listener.reset(s, Condition::Readable | Condition::Writable);
+    listener.backend().onUnset = [&] (auto flags, auto removal) {
+        ASSERT_EQ(Condition::All, flags);
+        ASSERT_TRUE(removal);
+    };
     listener.remove(s);
 
     ASSERT_EQ(0U, listener.size());
-    ASSERT_TRUE(listener.backend().m_isset);
-    ASSERT_TRUE(listener.backend().m_isunset);
-    ASSERT_TRUE(listener.backend().m_flags == Condition::None);
-    ASSERT_TRUE(listener.backend().m_removal);
 }
 
 TEST(ListenerUnsetRemove, failure)