From 53c5bd3be9cb15eca1cdfa4fa3e83db02ac7cca5 Mon Sep 17 00:00:00 2001 From: Michael Spang Date: Tue, 9 Aug 2022 21:36:29 -0400 Subject: [PATCH] Fix compilation of transport/SessionHandle.h (#21765) Currently SessionHandle, SessionHolder, and Session must be defined in order. Any other order will not compile. Merge these headers in order to satisfy the following objectives: 1) All headers compile in isolation 2) Header order inclusion does not matter This is the simplest way to fix the problem, and seems to be the only way to fix it without changing the class definitions. --- src/transport/Session.h | 126 +++++++++++++++++++++++++++++++- src/transport/SessionHandle.cpp | 8 -- src/transport/SessionHandle.h | 40 +--------- src/transport/SessionHolder.h | 96 +----------------------- 4 files changed, 127 insertions(+), 143 deletions(-) diff --git a/src/transport/Session.h b/src/transport/Session.h index a8653691d3fc52..1e8daac22373a6 100644 --- a/src/transport/Session.h +++ b/src/transport/Session.h @@ -16,16 +16,140 @@ #pragma once +#include #include #include +#include #include #include +#include +#include #include #include -#include +#include #include namespace chip { +namespace Transport { +class Session; +} // namespace Transport + +/** @brief + * Non-copyable session reference. All SessionHandles are created within SessionManager. It is not allowed to store SessionHandle + * anywhere except for function arguments and return values. + * + * SessionHandle is reference counted such that it is never dangling, but there can be a gray period when the session is marked + * as pending removal but not actually removed yet. During this period, the handle is functional, but the underlying session + * won't be able to be grabbed by any SessionHolder. SessionHandle->IsActiveSession can be used to check if the session is + * active. + */ +class SessionHandle +{ +public: + SessionHandle(Transport::Session & session) : mSession(session) {} + ~SessionHandle() {} + + SessionHandle(const SessionHandle &) = delete; + SessionHandle operator=(const SessionHandle &) = delete; + SessionHandle(SessionHandle &&) = default; + SessionHandle & operator=(SessionHandle &&) = delete; + + bool operator==(const SessionHandle & that) const { return &mSession.Get() == &that.mSession.Get(); } + + Transport::Session * operator->() const { return mSession.operator->(); } + +private: + friend class SessionHolder; + ReferenceCountedHandle mSession; +}; + +/** @brief + * Managed session reference. The object is used to store a session, the stored session will be automatically + * released when the underlying session is released. One must verify it is available before use. The object can be + * created using SessionHandle.Grab() + */ +class SessionHolder : public IntrusiveListNodeBase<> +{ +public: + SessionHolder() {} + SessionHolder(const SessionHandle & handle) { Grab(handle); } + virtual ~SessionHolder(); + + SessionHolder(const SessionHolder &); + SessionHolder(SessionHolder && that); + SessionHolder & operator=(const SessionHolder &); + SessionHolder & operator=(SessionHolder && that); + + virtual void SessionReleased() { Release(); } + virtual void ShiftToSession(const SessionHandle & session) + { + Release(); + Grab(session); + } + + bool Contains(const SessionHandle & session) const + { + return mSession.HasValue() && &mSession.Value().Get() == &session.mSession.Get(); + } + + bool GrabPairingSession(const SessionHandle & session); // Should be only used inside CASE/PASE pairing. + bool Grab(const SessionHandle & session); + void Release(); + + explicit operator bool() const { return mSession.HasValue(); } + Optional Get() const + { + // + // We cannot return mSession directly even if Optional is internally composed of the same bits, + // since they are not actually equivalent type-wise, and SessionHandle does not permit copy-construction. + // + // So, construct a new Optional from the underlying Transport::Session reference. + // + return mSession.HasValue() ? chip::MakeOptional(mSession.Value().Get()) + : chip::Optional::Missing(); + } + + Transport::Session * operator->() const { return &mSession.Value().Get(); } + + // There is not delegate, nothing to do here + virtual void DispatchSessionEvent(SessionDelegate::Event event) {} + +protected: + // Helper for use by the Grab methods. + void GrabUnchecked(const SessionHandle & session); + + Optional> mSession; +}; + +/// @brief Extends SessionHolder to allow propagate SessionDelegate::* events to a given destination +class SessionHolderWithDelegate : public SessionHolder +{ +public: + SessionHolderWithDelegate(SessionDelegate & delegate) : mDelegate(delegate) {} + SessionHolderWithDelegate(const SessionHandle & handle, SessionDelegate & delegate) : SessionHolder(handle), mDelegate(delegate) + {} + operator bool() const { return SessionHolder::operator bool(); } + + void SessionReleased() override + { + Release(); + + // Note, the session is already cleared during mDelegate.OnSessionReleased + mDelegate.OnSessionReleased(); + } + + void ShiftToSession(const SessionHandle & session) override + { + if (mDelegate.GetNewSessionHandlingPolicy() == SessionDelegate::NewSessionHandlingPolicy::kShiftToNewSession) + SessionHolder::ShiftToSession(session); + } + + void DispatchSessionEvent(SessionDelegate::Event event) override { (mDelegate.*event)(); } + +private: + SessionDelegate & mDelegate; +}; + namespace Transport { class SecureSession; diff --git a/src/transport/SessionHandle.cpp b/src/transport/SessionHandle.cpp index 3fe0ebfc1ff4b0..d31489c3b30599 100644 --- a/src/transport/SessionHandle.cpp +++ b/src/transport/SessionHandle.cpp @@ -15,12 +15,4 @@ * limitations under the License. */ -#include #include -#include - -namespace chip { - -using namespace Transport; - -} // namespace chip diff --git a/src/transport/SessionHandle.h b/src/transport/SessionHandle.h index 528d9b973f1212..c1b0aeeee0cb40 100644 --- a/src/transport/SessionHandle.h +++ b/src/transport/SessionHandle.h @@ -17,42 +17,4 @@ #pragma once -#include -#include - -namespace chip { - -namespace Transport { -class Session; -} // namespace Transport - -/** @brief - * Non-copyable session reference. All SessionHandles are created within SessionManager. It is not allowed to store SessionHandle - * anywhere except for function arguments and return values. - * - * SessionHandle is reference counted such that it is never dangling, but there can be a gray period when the session is marked - * as pending removal but not actually removed yet. During this period, the handle is functional, but the underlying session - * won't be able to be grabbed by any SessionHolder. SessionHandle->IsActiveSession can be used to check if the session is - * active. - */ -class SessionHandle -{ -public: - SessionHandle(Transport::Session & session) : mSession(session) {} - ~SessionHandle() {} - - SessionHandle(const SessionHandle &) = delete; - SessionHandle operator=(const SessionHandle &) = delete; - SessionHandle(SessionHandle &&) = default; - SessionHandle & operator=(SessionHandle &&) = delete; - - bool operator==(const SessionHandle & that) const { return &mSession.Get() == &that.mSession.Get(); } - - Transport::Session * operator->() const { return mSession.operator->(); } - -private: - friend class SessionHolder; - ReferenceCountedHandle mSession; -}; - -} // namespace chip +#include diff --git a/src/transport/SessionHolder.h b/src/transport/SessionHolder.h index d81b7c6256a4e5..55eb594255eab6 100644 --- a/src/transport/SessionHolder.h +++ b/src/transport/SessionHolder.h @@ -16,98 +16,4 @@ #pragma once -#include -#include -#include -#include - -namespace chip { - -/** @brief - * Managed session reference. The object is used to store a session, the stored session will be automatically - * released when the underlying session is released. One must verify it is available before use. The object can be - * created using SessionHandle.Grab() - */ -class SessionHolder : public IntrusiveListNodeBase<> -{ -public: - SessionHolder() {} - SessionHolder(const SessionHandle & handle) { Grab(handle); } - virtual ~SessionHolder(); - - SessionHolder(const SessionHolder &); - SessionHolder(SessionHolder && that); - SessionHolder & operator=(const SessionHolder &); - SessionHolder & operator=(SessionHolder && that); - - virtual void SessionReleased() { Release(); } - virtual void ShiftToSession(const SessionHandle & session) - { - Release(); - Grab(session); - } - - bool Contains(const SessionHandle & session) const - { - return mSession.HasValue() && &mSession.Value().Get() == &session.mSession.Get(); - } - - bool GrabPairingSession(const SessionHandle & session); // Should be only used inside CASE/PASE pairing. - bool Grab(const SessionHandle & session); - void Release(); - - explicit operator bool() const { return mSession.HasValue(); } - Optional Get() const - { - // - // We cannot return mSession directly even if Optional is internally composed of the same bits, - // since they are not actually equivalent type-wise, and SessionHandle does not permit copy-construction. - // - // So, construct a new Optional from the underlying Transport::Session reference. - // - return mSession.HasValue() ? chip::MakeOptional(mSession.Value().Get()) - : chip::Optional::Missing(); - } - - Transport::Session * operator->() const { return &mSession.Value().Get(); } - - // There is not delegate, nothing to do here - virtual void DispatchSessionEvent(SessionDelegate::Event event) {} - -protected: - // Helper for use by the Grab methods. - void GrabUnchecked(const SessionHandle & session); - - Optional> mSession; -}; - -/// @brief Extends SessionHolder to allow propagate SessionDelegate::* events to a given destination -class SessionHolderWithDelegate : public SessionHolder -{ -public: - SessionHolderWithDelegate(SessionDelegate & delegate) : mDelegate(delegate) {} - SessionHolderWithDelegate(const SessionHandle & handle, SessionDelegate & delegate) : SessionHolder(handle), mDelegate(delegate) - {} - operator bool() const { return SessionHolder::operator bool(); } - - void SessionReleased() override - { - Release(); - - // Note, the session is already cleared during mDelegate.OnSessionReleased - mDelegate.OnSessionReleased(); - } - - void ShiftToSession(const SessionHandle & session) override - { - if (mDelegate.GetNewSessionHandlingPolicy() == SessionDelegate::NewSessionHandlingPolicy::kShiftToNewSession) - SessionHolder::ShiftToSession(session); - } - - void DispatchSessionEvent(SessionDelegate::Event event) override { (mDelegate.*event)(); } - -private: - SessionDelegate & mDelegate; -}; - -} // namespace chip +#include