Skip to content

Commit 3ee7a14

Browse files
committed
Address some review feedback
1 parent 58953c6 commit 3ee7a14

File tree

6 files changed

+39
-32
lines changed

6 files changed

+39
-32
lines changed

src/app/server-cluster/ServerClusterInterface.h

+34-27
Original file line numberDiff line numberDiff line change
@@ -53,24 +53,16 @@ class IntrusiveSingleLinkedList
5353
{
5454
other.SetNotInList();
5555
}
56-
IntrusiveSingleLinkedList(const IntrusiveSingleLinkedList & other) :
57-
mNext((other.mNext == &other) ? static_cast<SELF *>(this) : other.mNext)
58-
{}
59-
IntrusiveSingleLinkedList & operator=(const IntrusiveSingleLinkedList & other)
60-
{
61-
if (&other != this)
62-
{
63-
mNext = (other.mNext == &other) ? static_cast<SELF *>(this) : other.mNext;
64-
}
65-
return *this;
66-
}
6756
IntrusiveSingleLinkedList & operator=(IntrusiveSingleLinkedList && other)
6857
{
6958
mNext = (other.mNext == &other) ? static_cast<SELF *>(this) : other.mNext;
7059
other.SetNotInList();
7160
return *this;
7261
}
7362

63+
IntrusiveSingleLinkedList(const IntrusiveSingleLinkedList & other) = delete;
64+
IntrusiveSingleLinkedList & operator=(const IntrusiveSingleLinkedList & other) = delete;
65+
7466
/// Determines whether this object is part of a linked list already.
7567
[[nodiscard]] bool IsInList() const { return (mNext != this); }
7668

@@ -85,8 +77,17 @@ class IntrusiveSingleLinkedList
8577
return mNext;
8678
}
8779

88-
/// Sets the "next" pointer when the SELF is assumed to be
89-
/// part of a SINGLE linked list.
80+
/// Sets the "next" pointer.
81+
///
82+
/// A `SELF` has a `next` pointer when it is part of a linked list.
83+
/// `SELF` is only allowed to be part of a SINGLE linked list at
84+
/// one time (there is only one `next`).
85+
///
86+
/// `value` MUST be an element that already is part of a single linked
87+
/// list (or nullptr).
88+
///
89+
/// NOTE: the marker of `next == this` is a flag that marks
90+
/// that `SELF` is not part of a linked list.
9091
///
9192
/// Returns the old value of "next"
9293
SELF * SetNextListItem(SELF * value)
@@ -107,7 +108,10 @@ class IntrusiveSingleLinkedList
107108

108109
} // namespace detail
109110

110-
/// Defines an active cluster on an endpoint.
111+
/// Handles cluster interactions for a specific cluster id.
112+
///
113+
/// A `ServerClusterInterface` is generally associated with a single endpoint id and represents
114+
/// a cluster that exists at a given `endpoint_id/cluster_id` path.
111115
///
112116
/// Provides metadata as well as interaction processing (attribute read/write and command handling).
113117
///
@@ -122,16 +126,16 @@ class ServerClusterInterface : public detail::IntrusiveSingleLinkedList<ServerCl
122126
ServerClusterInterface() = default;
123127
virtual ~ServerClusterInterface() = default;
124128

125-
ServerClusterInterface(const ServerClusterInterface & other) = default;
126129
ServerClusterInterface(ServerClusterInterface && other) = default;
127-
ServerClusterInterface & operator=(const ServerClusterInterface & other) = default;
128130
ServerClusterInterface & operator=(ServerClusterInterface && other) = default;
129131

132+
ServerClusterInterface(const ServerClusterInterface & other) = delete;
133+
ServerClusterInterface & operator=(const ServerClusterInterface & other) = delete;
134+
130135
///////////////////////////////////// Cluster Metadata Support //////////////////////////////////////////////////
131136
[[nodiscard]] virtual ClusterId GetClusterId() const = 0;
132137

133-
// Every cluster instance must have a data version. Base class implementation to avoid
134-
// code duplication
138+
// Every cluster instance must have a data version.
135139
//
136140
// SPEC - 7.10.3. Cluster Data Version
137141
// A cluster data version is a metadata increment-only counter value, maintained for each cluster instance.
@@ -144,19 +148,19 @@ class ServerClusterInterface : public detail::IntrusiveSingleLinkedList<ServerCl
144148
//
145149
[[nodiscard]] virtual DataVersion GetDataVersion() const = 0;
146150

147-
/// Cluster flags can be overridden, however most clusters likely have a default of "nothing special".
148-
///
149-
/// Default implementation returns a 0/empty quality list.
150151
[[nodiscard]] virtual BitFlags<DataModel::ClusterQualityFlags> GetClusterFlags() const = 0;
151152

152153
///////////////////////////////////// Attribute Support ////////////////////////////////////////////////////////
153154

154-
/// ReadAttribute MUST be done on a valid attribute path. `request.path` is expected to have `GetClusterId` as the cluster
155-
/// id as well as an attribute that is included in a `Attributes` call.
155+
/// ReadAttribute MUST be done on an "existent" attribute path: only on attributes that are
156+
/// returned in an `Attributes` call for this cluster.
157+
///
158+
/// `request.path` is expected to have `GetClusterId` as the cluster id as well as an attribute that is
159+
/// included in a `Attributes` call.
156160
///
157161
/// This MUST HANDLE the following global attributes:
158-
/// - FeatureMap::Id - generally 0 as a default
159-
/// - ClusterRevision::Id - this is implementation defined
162+
/// - FeatureMap::Id
163+
/// - ClusterRevision::Id
160164
///
161165
/// This function WILL NOT be called for attributes that can be built out of cluster metadata.
162166
/// Specifically this WILL NOT be called (and does not need to implement handling for) the
@@ -167,8 +171,11 @@ class ServerClusterInterface : public detail::IntrusiveSingleLinkedList<ServerCl
167171
virtual DataModel::ActionReturnStatus ReadAttribute(const DataModel::ReadAttributeRequest & request,
168172
AttributeValueEncoder & encoder) = 0;
169173

170-
/// WriteAttribute MUST be done on a valid attribute path. `request.path` is expected to have `GetClusterId` as the cluster
171-
/// id as well as an attribute that is included in a `Attributes` call.
174+
/// WriteAttribute MUST be done on an "existent" attribute path: only on attributes that are
175+
/// returned in an `Attributes` call for this cluster.
176+
///
177+
/// `request.path` is expected to have `GetClusterId` as the cluster id as well as an attribute that is
178+
/// included in a `Attributes` call.
172179
virtual DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request,
173180
AttributeValueDecoder & decoder) = 0;
174181

third_party/imgui/repo

Submodule repo updated 138 files

third_party/lwip/repo

Submodule repo updated 107 files

third_party/mbedtls/repo

Submodule repo updated from 0834c59 to 5bc604f

0 commit comments

Comments
 (0)