From a002b5fce8ff3673ff674502d74f03480b2037eb Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 28 Oct 2024 22:03:53 -0400 Subject: [PATCH] Factor out common parts of list iterators into shared super-classes. Saves some codesize. Fixes https://github.com/project-chip/connectedhomeip/issues/36264 --- src/app/data-model/DecodableList.h | 184 ++++++++++++++++++++--------- 1 file changed, 128 insertions(+), 56 deletions(-) diff --git a/src/app/data-model/DecodableList.h b/src/app/data-model/DecodableList.h index b05db43c1522bf..b65bf674960885 100644 --- a/src/app/data-model/DecodableList.h +++ b/src/app/data-model/DecodableList.h @@ -83,8 +83,103 @@ class DecodableListBase } protected: + class Iterator + { + public: + Iterator(const TLV::TLVReader & reader) + { + mStatus = CHIP_NO_ERROR; + mReader.Init(reader); + } + + bool Next() + { + if (mReader.GetContainerType() == TLV::kTLVType_NotSpecified) + { + return false; + } + + if (mStatus == CHIP_NO_ERROR) + { + mStatus = mReader.Next(); + } + + return (mStatus == CHIP_NO_ERROR); + } + + /* + * Returns the result of all previous operations on this iterator. + * + * Notably, if the end-of-list was encountered in a previous call to Next, + * the status returned shall be CHIP_NO_ERROR. + */ + CHIP_ERROR GetStatus() const + { + if (mStatus == CHIP_END_OF_TLV) + { + return CHIP_NO_ERROR; + } + + return mStatus; + } + + protected: + CHIP_ERROR mStatus; + TLV::TLVReader mReader; + }; + TLV::TLVReader mReader; - chip::Optional mFabricIndex; +}; + +template +class FabricIndexListMemberMixin +{ +}; + +template <> +class FabricIndexListMemberMixin +{ +public: + void SetFabricIndex(FabricIndex fabricIndex) { mFabricIndex.SetValue(fabricIndex); } + +protected: + Optional mFabricIndex; +}; + +template +class FabricIndexIteratorMemberMixin +{ +}; + +template <> +class FabricIndexIteratorMemberMixin +{ +public: + FabricIndexIteratorMemberMixin(const Optional & fabricindex) : mFabricIndex(fabricindex) {} + +protected: + const Optional mFabricIndex; +}; + +template +class DecodableMaybeFabricScopedList : public DecodableListBase, public FabricIndexListMemberMixin +{ +public: + static constexpr bool kIsFabricScoped = IsFabricScoped; + +protected: + class Iterator : public DecodableListBase::Iterator, public FabricIndexIteratorMemberMixin + { + public: + template = true> + Iterator(const TLV::TLVReader & reader, const Optional & fabricIndex) : + DecodableListBase::Iterator(reader), FabricIndexIteratorMemberMixin(fabricIndex) + {} + + template = true> + Iterator(const TLV::TLVReader & reader) : DecodableListBase::Iterator(reader) + {} + }; }; } // namespace detail @@ -109,21 +204,15 @@ class DecodableListBase * */ template -class DecodableList : public detail::DecodableListBase +class DecodableList : public detail::DecodableMaybeFabricScopedList::value> { public: DecodableList() {} - static constexpr bool kIsFabricScoped = DataModel::IsFabricScoped::value; - - template ::value, bool> = true> - void SetFabricIndex(FabricIndex fabricIndex) + class Iterator : public detail::DecodableMaybeFabricScopedList::value>::Iterator { - mFabricIndex.SetValue(fabricIndex); - } + using IteratorBase = typename detail::DecodableMaybeFabricScopedList::value>::Iterator; - class Iterator - { public: /* * Initialize the iterator with a reference to a reader. @@ -133,11 +222,13 @@ class DecodableList : public detail::DecodableListBase * have a `kTLVType_NotSpecified` container type if there is * no list. */ - Iterator(const TLV::TLVReader & reader, Optional fabricIndex) : mFabricIndex(fabricIndex) - { - mStatus = CHIP_NO_ERROR; - mReader.Init(reader); - } + template ::value, bool> = true> + Iterator(const TLV::TLVReader & reader, Optional fabricIndex) : IteratorBase(reader, fabricIndex) + {} + + template ::value, bool> = true> + Iterator(const TLV::TLVReader & reader) : IteratorBase(reader) + {} /* * Increments the iterator to point to the next list element @@ -157,17 +248,17 @@ class DecodableList : public detail::DecodableListBase template ::value, bool> = true> bool Next() { - return DoNext(); + return IteratorBase::Next() && DecodeValue(); } template ::value, bool> = true> bool Next() { - bool hasNext = DoNext(); + bool hasNext = IteratorBase::Next() && DecodeValue(); - if (hasNext && mFabricIndex.HasValue()) + if (hasNext && this->mFabricIndex.HasValue()) { - mValue.SetFabricIndex(mFabricIndex.Value()); + mValue.SetFabricIndex(this->mFabricIndex.Value()); } return hasNext; @@ -179,36 +270,10 @@ class DecodableList : public detail::DecodableListBase */ const T & GetValue() const { return mValue; } - /* - * Returns the result of all previous operations on this iterator. - * - * Notably, if the end-of-list was encountered in a previous call to Next, - * the status returned shall be CHIP_NO_ERROR. - */ - CHIP_ERROR GetStatus() const - { - if (mStatus == CHIP_END_OF_TLV) - { - return CHIP_NO_ERROR; - } - - return mStatus; - } - private: - bool DoNext() + bool DecodeValue() { - if (mReader.GetContainerType() == TLV::kTLVType_NotSpecified) - { - return false; - } - - if (mStatus == CHIP_NO_ERROR) - { - mStatus = mReader.Next(); - } - - if (mStatus == CHIP_NO_ERROR) + if (this->mStatus == CHIP_NO_ERROR) { // // Re-construct mValue to reset its state back to cluster object defaults. @@ -218,22 +283,29 @@ class DecodableList : public detail::DecodableListBase // data from previous decode attempts will continue to linger and give // an incorrect view of the state as seen from a client. // - mValue = T(); - mStatus = DataModel::Decode(mReader, mValue); + mValue = T(); + this->mStatus = DataModel::Decode(this->mReader, mValue); } - return (mStatus == CHIP_NO_ERROR); + return (this->mStatus == CHIP_NO_ERROR); } T mValue; - CHIP_ERROR mStatus; - TLV::TLVReader mReader; - // TODO: Consider some setup where this field does not exist when T - // is not a fabric scoped struct. - const Optional mFabricIndex; }; - Iterator begin() const { return Iterator(mReader, mFabricIndex); } + // Need this->mReader and this->mFabricIndex for the name lookup to realize + // those can be found in our superclasses. + template ::value, bool> = true> + Iterator begin() const + { + return Iterator(this->mReader, this->mFabricIndex); + } + + template ::value, bool> = true> + Iterator begin() const + { + return Iterator(this->mReader); + } }; } // namespace DataModel