Skip to content

Commit

Permalink
Make ReadClient::Callback non-movable (#29191)
Browse files Browse the repository at this point in the history
The purpose of this interface is be called polymorphically through a
pointer. Moving it will invalidate the original instance and is very
likely to be a bug.

Make this type non-movable; this will fail compilation if a derived
class is stored in an inappropriate container that moves its elements.

As an example, ClusterStateCache is also currently movable, but moving
it retains a pointer to the moved-from instance via mBufferedReader, so
any further usage is likely to result in a invalid memory access.
  • Loading branch information
mspang authored and pull[bot] committed Dec 29, 2023
1 parent bd59f53 commit 1516956
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
13 changes: 13 additions & 0 deletions src/app/ClusterStateCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ class ClusterStateCache : protected ReadClient::Callback
class Callback : public ReadClient::Callback
{
public:
Callback() = default;

// Callbacks are not expected to be copyable or movable.
Callback(const Callback &) = delete;
Callback(Callback &&) = delete;
Callback & operator=(const Callback &) = delete;
Callback & operator=(Callback &&) = delete;

/*
* Called anytime an attribute value has changed in the cache
*/
Expand Down Expand Up @@ -103,6 +111,11 @@ class ClusterStateCache : protected ReadClient::Callback
mHighestReceivedEventNumber = highestReceivedEventNumber;
}

ClusterStateCache(const ClusterStateCache &) = delete;
ClusterStateCache(ClusterStateCache &&) = delete;
ClusterStateCache & operator=(const ClusterStateCache &) = delete;
ClusterStateCache & operator=(ClusterStateCache &&) = delete;

void SetHighestReceivedEventNumber(EventNumber highestReceivedEventNumber)
{
mHighestReceivedEventNumber.SetValue(highestReceivedEventNumber);
Expand Down
1 change: 0 additions & 1 deletion src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ ReadClient::ReadClient(InteractionModelEngine * apImEngine, Messaging::ExchangeM
mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this)
{
mpExchangeMgr = apExchangeMgr;
mpCallback = apCallback;
mInteractionType = aInteractionType;

mpImEngine = apImEngine;
Expand Down
8 changes: 8 additions & 0 deletions src/app/ReadClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ class ReadClient : public Messaging::ExchangeDelegate
class Callback
{
public:
Callback() = default;

// Callbacks are not expected to be copyable or movable.
Callback(const Callback &) = delete;
Callback(Callback &&) = delete;
Callback & operator=(const Callback &) = delete;
Callback & operator=(Callback &&) = delete;

virtual ~Callback() = default;

/**
Expand Down

0 comments on commit 1516956

Please sign in to comment.