Skip to content

Commit

Permalink
Fix SessionHandle args so they are const ref (#13053)
Browse files Browse the repository at this point in the history
They were being passed by const ref in some cases, but also by value,
and in a few cases by const value.
  • Loading branch information
mlepage-google authored and pull[bot] committed Nov 15, 2023
1 parent 414214b commit e43d693
Show file tree
Hide file tree
Showing 38 changed files with 103 additions and 95 deletions.
4 changes: 2 additions & 2 deletions src/app/CASESessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@ CHIP_ERROR CASESessionManager::GetPeerAddress(FabricInfo * fabric, NodeId nodeId
return CHIP_NO_ERROR;
}

void CASESessionManager::OnSessionReleased(SessionHandle sessionHandle)
void CASESessionManager::OnSessionReleased(const SessionHandle & sessionHandle)
{
OperationalDeviceProxy * session = FindSession(sessionHandle);
VerifyOrReturn(session != nullptr, ChipLogDetail(Controller, "OnSessionReleased was called for unknown device, ignoring it."));

session->OnSessionReleased(sessionHandle);
}

OperationalDeviceProxy * CASESessionManager::FindSession(SessionHandle session)
OperationalDeviceProxy * CASESessionManager::FindSession(const SessionHandle & session)
{
return mConfig.devicePool->FindDevice(session);
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/CASESessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,15 @@ class CASESessionManager : public SessionReleaseDelegate, public Dnssd::Resolver
CHIP_ERROR GetPeerAddress(FabricInfo * fabric, NodeId nodeId, Transport::PeerAddress & addr);

//////////// SessionReleaseDelegate Implementation ///////////////
void OnSessionReleased(SessionHandle session) override;
void OnSessionReleased(const SessionHandle & session) override;

//////////// ResolverDelegate Implementation ///////////////
void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) override;
void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override;
void OnNodeDiscoveryComplete(const Dnssd::DiscoveredNodeData & nodeData) override {}

private:
OperationalDeviceProxy * FindSession(SessionHandle session);
OperationalDeviceProxy * FindSession(const SessionHandle & session);
void ReleaseSession(OperationalDeviceProxy * device);

CASESessionManagerConfig mConfig;
Expand Down
2 changes: 1 addition & 1 deletion src/app/CommandSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ CHIP_ERROR CommandSender::AllocateBuffer()
return CHIP_NO_ERROR;
}

CHIP_ERROR CommandSender::SendCommandRequest(SessionHandle session, System::Clock::Timeout timeout)
CHIP_ERROR CommandSender::SendCommandRequest(const SessionHandle & session, System::Clock::Timeout timeout)
{
VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE);

Expand Down
2 changes: 1 addition & 1 deletion src/app/CommandSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class CommandSender final : public Messaging::ExchangeDelegate
// Client can specify the maximum time to wait for response (in milliseconds) via timeout parameter.
// Default timeout value will be used otherwise.
//
CHIP_ERROR SendCommandRequest(SessionHandle session, System::Clock::Timeout timeout = kImMessageTimeout);
CHIP_ERROR SendCommandRequest(const SessionHandle & session, System::Clock::Timeout timeout = kImMessageTimeout);

private:
friend class TestCommandInteraction;
Expand Down
2 changes: 1 addition & 1 deletion src/app/OperationalDeviceProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ void OperationalDeviceProxy::DeferCloseCASESession()
mSystemLayer->ScheduleWork(CloseCASESessionTask, this);
}

void OperationalDeviceProxy::OnSessionReleased(SessionHandle session)
void OperationalDeviceProxy::OnSessionReleased(const SessionHandle & session)
{
VerifyOrReturn(mSecureSession.Contains(session),
ChipLogDetail(Controller, "Connection expired, but it doesn't match the current session"));
Expand Down
4 changes: 2 additions & 2 deletions src/app/OperationalDeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
* Called when a connection is closing.
* The object releases all resources associated with the connection.
*/
void OnSessionReleased(SessionHandle session) override;
void OnSessionReleased(const SessionHandle & session) override;

void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeResolutionData)
{
Expand Down Expand Up @@ -155,7 +155,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele

PeerId GetPeerId() const { return mPeerId; }

bool MatchesSession(SessionHandle session) const { return mSecureSession.Contains(session); }
bool MatchesSession(const SessionHandle & session) const { return mSecureSession.Contains(session); }

uint8_t GetNextSequenceNumber() override { return mSequenceNumber++; };

Expand Down
4 changes: 2 additions & 2 deletions src/app/OperationalDeviceProxyPool.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class OperationalDeviceProxyPoolDelegate

virtual void Release(OperationalDeviceProxy * device) = 0;

virtual OperationalDeviceProxy * FindDevice(SessionHandle session) = 0;
virtual OperationalDeviceProxy * FindDevice(const SessionHandle & session) = 0;

virtual OperationalDeviceProxy * FindDevice(NodeId id) = 0;

Expand All @@ -59,7 +59,7 @@ class OperationalDeviceProxyPool : public OperationalDeviceProxyPoolDelegate

void Release(OperationalDeviceProxy * device) override { mDevicePool.ReleaseObject(device); }

OperationalDeviceProxy * FindDevice(SessionHandle session) override
OperationalDeviceProxy * FindDevice(const SessionHandle & session) override
{
OperationalDeviceProxy * foundDevice = nullptr;
mDevicePool.ForEachActiveObject([&](auto * activeDevice) {
Expand Down
2 changes: 1 addition & 1 deletion src/app/ReadPrepareParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ struct ReadPrepareParams
uint16_t mMaxIntervalCeilingSeconds = 0;
bool mKeepSubscriptions = true;

ReadPrepareParams(SessionHandle sessionHandle) : mSessionHandle(sessionHandle) {}
ReadPrepareParams(const SessionHandle & sessionHandle) : mSessionHandle(sessionHandle) {}
ReadPrepareParams(ReadPrepareParams && other) : mSessionHandle(other.mSessionHandle)
{
mKeepSubscriptions = other.mKeepSubscriptions;
Expand Down
4 changes: 2 additions & 2 deletions src/app/WriteClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ void WriteClient::ClearState()
MoveToState(State::Uninitialized);
}

CHIP_ERROR WriteClient::SendWriteRequest(SessionHandle session, System::Clock::Timeout timeout)
CHIP_ERROR WriteClient::SendWriteRequest(const SessionHandle & session, System::Clock::Timeout timeout)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand Down Expand Up @@ -385,7 +385,7 @@ CHIP_ERROR WriteClient::ProcessAttributeStatusIB(AttributeStatusIB::Parser & aAt
return err;
}

CHIP_ERROR WriteClientHandle::SendWriteRequest(SessionHandle session, System::Clock::Timeout timeout)
CHIP_ERROR WriteClientHandle::SendWriteRequest(const SessionHandle & session, System::Clock::Timeout timeout)
{
CHIP_ERROR err = mpWriteClient->SendWriteRequest(session, timeout);

Expand Down
4 changes: 2 additions & 2 deletions src/app/WriteClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class WriteClient : public Messaging::ExchangeDelegate
* If SendWriteRequest is never called, or the call fails, the API
* consumer is responsible for calling Shutdown on the WriteClient.
*/
CHIP_ERROR SendWriteRequest(SessionHandle session, System::Clock::Timeout timeout);
CHIP_ERROR SendWriteRequest(const SessionHandle & session, System::Clock::Timeout timeout);

/**
* Initialize the client object. Within the lifetime
Expand Down Expand Up @@ -249,7 +249,7 @@ class WriteClientHandle
* Finalize the message and send it to the desired node. The underlying write object will always be released, and the user
* should not use this object after calling this function.
*/
CHIP_ERROR SendWriteRequest(SessionHandle session, System::Clock::Timeout timeout = kImMessageTimeout);
CHIP_ERROR SendWriteRequest(const SessionHandle & session, System::Clock::Timeout timeout = kImMessageTimeout);

/**
* Encode an attribute value that can be directly encoded using TLVWriter::Put
Expand Down
6 changes: 3 additions & 3 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ void DeviceController::ReleaseOperationalDevice(NodeId remoteDeviceId)
mCASESessionManager->ReleaseSession(remoteDeviceId);
}

void DeviceController::OnSessionReleased(SessionHandle session)
void DeviceController::OnSessionReleased(const SessionHandle & session)
{
VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnConnectionExpired was called in incorrect state"));
mCASESessionManager->OnSessionReleased(session);
Expand Down Expand Up @@ -684,7 +684,7 @@ CHIP_ERROR DeviceCommissioner::Shutdown()
return CHIP_NO_ERROR;
}

void DeviceCommissioner::OnSessionReleased(SessionHandle session)
void DeviceCommissioner::OnSessionReleased(const SessionHandle & session)
{
VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnConnectionExpired was called in incorrect state"));

Expand All @@ -694,7 +694,7 @@ void DeviceCommissioner::OnSessionReleased(SessionHandle session)
device->OnSessionReleased(session);
}

CommissioneeDeviceProxy * DeviceCommissioner::FindCommissioneeDevice(SessionHandle session)
CommissioneeDeviceProxy * DeviceCommissioner::FindCommissioneeDevice(const SessionHandle & session)
{
CommissioneeDeviceProxy * foundDevice = nullptr;
mCommissioneeDevicePool.ForEachActiveObject([&](auto * deviceProxy) {
Expand Down
6 changes: 3 additions & 3 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ class DLL_EXPORT DeviceController : public SessionReleaseDelegate,
ReliableMessageProtocolConfig mMRPConfig = gDefaultMRPConfig;

//////////// SessionReleaseDelegate Implementation ///////////////
void OnSessionReleased(SessionHandle session) override;
void OnSessionReleased(const SessionHandle & session) override;

//////////// SessionRecoveryDelegate Implementation ///////////////
void OnFirstMessageDeliveryFailed(const SessionHandle & session) override;
Expand Down Expand Up @@ -682,7 +682,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
void OnSessionEstablishmentTimeout();

//////////// SessionReleaseDelegate Implementation ///////////////
void OnSessionReleased(SessionHandle session) override;
void OnSessionReleased(const SessionHandle & session) override;

static void OnSessionEstablishmentTimeoutCallback(System::Layer * aLayer, void * aAppState);

Expand Down Expand Up @@ -777,7 +777,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,

void HandleAttestationResult(CHIP_ERROR err);

CommissioneeDeviceProxy * FindCommissioneeDevice(SessionHandle session);
CommissioneeDeviceProxy * FindCommissioneeDevice(const SessionHandle & session);
CommissioneeDeviceProxy * FindCommissioneeDevice(NodeId id);
void ReleaseCommissioneeDevice(CommissioneeDeviceProxy * device);

Expand Down
2 changes: 1 addition & 1 deletion src/controller/CommissioneeDeviceProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ CHIP_ERROR CommissioneeDeviceProxy::SendCommands(app::CommandSender * commandObj
return commandObj->SendCommandRequest(mSecureSession.Get());
}

void CommissioneeDeviceProxy::OnSessionReleased(SessionHandle session)
void CommissioneeDeviceProxy::OnSessionReleased(const SessionHandle & session)
{
VerifyOrReturn(mSecureSession.Contains(session),
ChipLogDetail(Controller, "Connection expired, but it doesn't match the current session"));
Expand Down
4 changes: 2 additions & 2 deletions src/controller/CommissioneeDeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat
*
* @param session A handle to the secure session
*/
void OnSessionReleased(SessionHandle session) override;
void OnSessionReleased(const SessionHandle & session) override;

/**
* In case there exists an open session to the device, mark it as expired.
Expand Down Expand Up @@ -204,7 +204,7 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat

NodeId GetDeviceId() const override { return mDeviceId; }

bool MatchesSession(SessionHandle session) const { return mSecureSession.Contains(session); }
bool MatchesSession(const SessionHandle & session) const { return mSecureSession.Contains(session); }

SessionHolder & GetSecureSessionHolder() { return mSecureSession; }
chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession.ToOptional(); }
Expand Down
6 changes: 3 additions & 3 deletions src/controller/InvokeInteraction.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace Controller {
*/
template <typename RequestObjectT>
CHIP_ERROR
InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId,
InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, const SessionHandle & sessionHandle, chip::EndpointId endpointId,
const RequestObjectT & requestCommandData,
typename TypedCommandCallback<typename RequestObjectT::ResponseType>::OnSuccessCallbackType onSuccessCb,
typename TypedCommandCallback<typename RequestObjectT::ResponseType>::OnErrorCallbackType onErrorCb,
Expand Down Expand Up @@ -91,7 +91,7 @@ InvokeCommandRequest(Messaging::ExchangeManager * aExchangeMgr, SessionHandle se

template <typename RequestObjectT>
CHIP_ERROR
InvokeCommandRequest(Messaging::ExchangeManager * exchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId,
InvokeCommandRequest(Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle, chip::EndpointId endpointId,
const RequestObjectT & requestCommandData,
typename TypedCommandCallback<typename RequestObjectT::ResponseType>::OnSuccessCallbackType onSuccessCb,
typename TypedCommandCallback<typename RequestObjectT::ResponseType>::OnErrorCallbackType onErrorCb,
Expand All @@ -103,7 +103,7 @@ InvokeCommandRequest(Messaging::ExchangeManager * exchangeMgr, SessionHandle ses

template <typename RequestObjectT, typename std::enable_if_t<!RequestObjectT::MustUseTimedInvoke(), int> = 0>
CHIP_ERROR
InvokeCommandRequest(Messaging::ExchangeManager * exchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId,
InvokeCommandRequest(Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle, chip::EndpointId endpointId,
const RequestObjectT & requestCommandData,
typename TypedCommandCallback<typename RequestObjectT::ResponseType>::OnSuccessCallbackType onSuccessCb,
typename TypedCommandCallback<typename RequestObjectT::ResponseType>::OnErrorCallbackType onErrorCb)
Expand Down
19 changes: 11 additions & 8 deletions src/controller/ReadInteraction.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ namespace detail {
template <typename DecodableAttributeType>
struct ReportAttributeParams : public app::ReadPrepareParams
{
ReportAttributeParams(SessionHandle sessionHandle) : app::ReadPrepareParams(sessionHandle) { mKeepSubscriptions = false; }
ReportAttributeParams(const SessionHandle & sessionHandle) : app::ReadPrepareParams(sessionHandle)
{
mKeepSubscriptions = false;
}
typename TypedReadAttributeCallback<DecodableAttributeType>::OnSuccessCallbackType mOnReportCb;
typename TypedReadAttributeCallback<DecodableAttributeType>::OnErrorCallbackType mOnErrorCb;
typename TypedReadAttributeCallback<DecodableAttributeType>::OnSubscriptionEstablishedCallbackType
Expand Down Expand Up @@ -84,7 +87,7 @@ CHIP_ERROR ReportAttribute(Messaging::ExchangeManager * exchangeMgr, EndpointId
* basis, we have a helper that's just templated on the type.
*/
template <typename DecodableAttributeType>
CHIP_ERROR ReadAttribute(Messaging::ExchangeManager * exchangeMgr, const SessionHandle sessionHandle, EndpointId endpointId,
CHIP_ERROR ReadAttribute(Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle, EndpointId endpointId,
ClusterId clusterId, AttributeId attributeId,
typename TypedReadAttributeCallback<DecodableAttributeType>::OnSuccessCallbackType onSuccessCb,
typename TypedReadAttributeCallback<DecodableAttributeType>::OnErrorCallbackType onErrorCb)
Expand All @@ -106,7 +109,7 @@ CHIP_ERROR ReadAttribute(Messaging::ExchangeManager * exchangeMgr, const Session
*/
template <typename AttributeTypeInfo>
CHIP_ERROR
ReadAttribute(Messaging::ExchangeManager * exchangeMgr, const SessionHandle sessionHandle, EndpointId endpointId,
ReadAttribute(Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle, EndpointId endpointId,
typename TypedReadAttributeCallback<typename AttributeTypeInfo::DecodableType>::OnSuccessCallbackType onSuccessCb,
typename TypedReadAttributeCallback<typename AttributeTypeInfo::DecodableType>::OnErrorCallbackType onErrorCb)
{
Expand All @@ -117,7 +120,7 @@ ReadAttribute(Messaging::ExchangeManager * exchangeMgr, const SessionHandle sess

// Helper for SubscribeAttribute to reduce the amount of code generated.
template <typename DecodableAttributeType>
CHIP_ERROR SubscribeAttribute(Messaging::ExchangeManager * exchangeMgr, const SessionHandle sessionHandle, EndpointId endpointId,
CHIP_ERROR SubscribeAttribute(Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle, EndpointId endpointId,
ClusterId clusterId, AttributeId attributeId,
typename TypedReadAttributeCallback<DecodableAttributeType>::OnSuccessCallbackType onReportCb,
typename TypedReadAttributeCallback<DecodableAttributeType>::OnErrorCallbackType onErrorCb,
Expand All @@ -142,7 +145,7 @@ CHIP_ERROR SubscribeAttribute(Messaging::ExchangeManager * exchangeMgr, const Se
*/
template <typename AttributeTypeInfo>
CHIP_ERROR SubscribeAttribute(
Messaging::ExchangeManager * exchangeMgr, const SessionHandle sessionHandle, EndpointId endpointId,
Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle, EndpointId endpointId,
typename TypedReadAttributeCallback<typename AttributeTypeInfo::DecodableType>::OnSuccessCallbackType onReportCb,
typename TypedReadAttributeCallback<typename AttributeTypeInfo::DecodableType>::OnErrorCallbackType onErrorCb,
uint16_t aMinIntervalFloorSeconds, uint16_t aMaxIntervalCeilingSeconds,
Expand All @@ -159,7 +162,7 @@ namespace detail {
template <typename DecodableEventType>
struct ReportEventParams : public app::ReadPrepareParams
{
ReportEventParams(SessionHandle sessionHandle) : app::ReadPrepareParams(sessionHandle) { mKeepSubscriptions = false; }
ReportEventParams(const SessionHandle & sessionHandle) : app::ReadPrepareParams(sessionHandle) { mKeepSubscriptions = false; }
typename TypedReadEventCallback<DecodableEventType>::OnSuccessCallbackType mOnReportCb;
typename TypedReadEventCallback<DecodableEventType>::OnErrorCallbackType mOnErrorCb;
typename TypedReadEventCallback<DecodableEventType>::OnSubscriptionEstablishedCallbackType mOnSubscriptionEstablishedCb =
Expand Down Expand Up @@ -220,7 +223,7 @@ CHIP_ERROR ReportEvent(Messaging::ExchangeManager * apExchangeMgr, EndpointId en
* GetEventId() methods is expected to work.
*/
template <typename DecodableEventType>
CHIP_ERROR ReadEvent(Messaging::ExchangeManager * exchangeMgr, const SessionHandle sessionHandle, EndpointId endpointId,
CHIP_ERROR ReadEvent(Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle, EndpointId endpointId,
typename TypedReadEventCallback<DecodableEventType>::OnSuccessCallbackType onSuccessCb,
typename TypedReadEventCallback<DecodableEventType>::OnErrorCallbackType onErrorCb)
{
Expand All @@ -235,7 +238,7 @@ CHIP_ERROR ReadEvent(Messaging::ExchangeManager * exchangeMgr, const SessionHand
* similarly to ReadEvent but keeps reporting events as they are emitted.
*/
template <typename DecodableEventType>
CHIP_ERROR SubscribeEvent(Messaging::ExchangeManager * exchangeMgr, const SessionHandle sessionHandle, EndpointId endpointId,
CHIP_ERROR SubscribeEvent(Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle, EndpointId endpointId,
typename TypedReadEventCallback<DecodableEventType>::OnSuccessCallbackType onReportCb,
typename TypedReadEventCallback<DecodableEventType>::OnErrorCallbackType onErrorCb,
uint16_t minIntervalFloorSeconds, uint16_t maxIntervalCeilingSeconds,
Expand Down
Loading

0 comments on commit e43d693

Please sign in to comment.