# HG changeset patch # User David Demelier # Date 1472555018 -7200 # Node ID 3058eaa603e67fd3ba27a2d81d488b836bbc17ca # Parent 5717cb946cd318b5bac7ab27078444a378576c73 Net: improve Listener::unset and its tests diff -r 5717cb946cd3 -r 3058eaa603e6 modules/net/net.hpp --- 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(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(condition) > 0x3) + return; + auto it = m_table.find(sc); - // Invalid or useless flags. - if (condition == Condition::None || static_cast(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); } /** diff -r 5717cb946cd3 -r 3058eaa603e6 modules/net/test/main.cpp --- 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 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 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 listener; + Listener 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 listener; + Listener 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 listener; + Listener 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 listener; + Listener 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)