Skip to content

Commit

Permalink
Use rvalue references for some PacketBufferHandle parameters
Browse files Browse the repository at this point in the history
#### Problem

Using `PacketBufferHandle` rather than raw `PacketBuffer*` imposes
some overhead on argument passing.

#### Summary of Changes

Parameters converted from by-value to by-rvalue-reference only where
the function unconditionally (and clearly) moves the handle out of
the parameter.

part of project-chip#2707 - Figure out a way to express PacketBuffer ownership in the type system
  • Loading branch information
kpschoedel committed Jan 13, 2021
1 parent 872015b commit 21318aa
Show file tree
Hide file tree
Showing 19 changed files with 43 additions and 43 deletions.
2 changes: 1 addition & 1 deletion src/app/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ CHIP_ERROR Command::Reset()
return err;
}

CHIP_ERROR Command::ProcessCommandMessage(System::PacketBufferHandle payload, CommandRoleId aCommandRoleId)
CHIP_ERROR Command::ProcessCommandMessage(System::PacketBufferHandle && payload, CommandRoleId aCommandRoleId)
{
CHIP_ERROR err = CHIP_NO_ERROR;
chip::System::PacketBufferTLVReader reader;
Expand Down
2 changes: 1 addition & 1 deletion src/app/Command.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class Command

protected:
void MoveToState(const CommandState aTargetState);
CHIP_ERROR ProcessCommandMessage(System::PacketBufferHandle payload, CommandRoleId aCommandRoleId);
CHIP_ERROR ProcessCommandMessage(System::PacketBufferHandle && payload, CommandRoleId aCommandRoleId);
void ClearState();
const char * GetStateStr() const;

Expand Down
10 changes: 5 additions & 5 deletions src/ble/BLEEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ BLE_ERROR BLEEndPoint::Init(BleLayer * bleLayer, BLE_CONNECTION_OBJECT connObj,
return err;
}

BLE_ERROR BLEEndPoint::SendCharacteristic(PacketBufferHandle buf)
BLE_ERROR BLEEndPoint::SendCharacteristic(PacketBufferHandle && buf)
{
BLE_ERROR err = BLE_NO_ERROR;

Expand Down Expand Up @@ -628,7 +628,7 @@ BLE_ERROR BLEEndPoint::SendCharacteristic(PacketBufferHandle buf)
* kType_Data(0) - data packet
* kType_Control(1) - control packet
*/
void BLEEndPoint::QueueTx(PacketBufferHandle data, PacketType_t type)
void BLEEndPoint::QueueTx(PacketBufferHandle && data, PacketType_t type)
{
#if CHIP_ENABLE_CHIPOBLE_TEST
ChipLogDebugBleEndPoint(Ble, "%s: data->%p, type %d, len %d", __FUNCTION__, data, type, data->DataLength());
Expand Down Expand Up @@ -690,7 +690,7 @@ BLE_ERROR BLEEndPoint::Send(PacketBufferHandle data)
return err;
}

bool BLEEndPoint::PrepareNextFragment(PacketBufferHandle data, bool & sentAck)
bool BLEEndPoint::PrepareNextFragment(PacketBufferHandle && data, bool & sentAck)
{
// If we have a pending fragment acknowledgement to send, piggyback it on the fragment we're about to transmit.
if (GetFlag(mTimerStateFlags, kTimerState_SendAckTimerRunning))
Expand Down Expand Up @@ -1451,14 +1451,14 @@ BLE_ERROR BLEEndPoint::Receive(PacketBufferHandle data)
return err;
}

bool BLEEndPoint::SendWrite(PacketBufferHandle buf)
bool BLEEndPoint::SendWrite(PacketBufferHandle && buf)
{
SetFlag(mConnStateFlags, kConnState_GattOperationInFlight, true);

return mBle->mPlatformDelegate->SendWriteRequest(mConnObj, &CHIP_BLE_SVC_ID, &mBle->CHIP_BLE_CHAR_1_ID, std::move(buf));
}

bool BLEEndPoint::SendIndication(PacketBufferHandle buf)
bool BLEEndPoint::SendIndication(PacketBufferHandle && buf)
{
SetFlag(mConnStateFlags, kConnState_GattOperationInFlight, true);

Expand Down
10 changes: 5 additions & 5 deletions src/ble/BLEEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,13 @@ class DLL_EXPORT BLEEndPoint : public BleLayerObject
// Transmit path:
BLE_ERROR DriveSending();
BLE_ERROR DriveStandAloneAck();
bool PrepareNextFragment(PacketBufferHandle data, bool & sentAck);
bool PrepareNextFragment(PacketBufferHandle && data, bool & sentAck);
BLE_ERROR SendNextMessage();
BLE_ERROR ContinueMessageSend();
BLE_ERROR DoSendStandAloneAck();
BLE_ERROR SendCharacteristic(PacketBufferHandle buf);
bool SendIndication(PacketBufferHandle buf);
bool SendWrite(PacketBufferHandle buf);
BLE_ERROR SendCharacteristic(PacketBufferHandle && buf);
bool SendIndication(PacketBufferHandle && buf);
bool SendWrite(PacketBufferHandle && buf);

// Receive path:
BLE_ERROR HandleConnectComplete();
Expand Down Expand Up @@ -223,7 +223,7 @@ class DLL_EXPORT BLEEndPoint : public BleLayerObject
inline void QueueTxLock() {}
inline void QueueTxUnlock() {}
#endif
void QueueTx(PacketBufferHandle data, PacketType_t type);
void QueueTx(PacketBufferHandle && data, PacketType_t type);
};

} /* namespace Ble */
Expand Down
6 changes: 3 additions & 3 deletions src/inet/RawEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ void RawEndPoint::Free()
* A synonym for <tt>SendTo(addr, INET_NULL_INTERFACEID, msg,
* sendFlags)</tt>.
*/
INET_ERROR RawEndPoint::SendTo(const IPAddress & addr, chip::System::PacketBufferHandle msg, uint16_t sendFlags)
INET_ERROR RawEndPoint::SendTo(const IPAddress & addr, chip::System::PacketBufferHandle && msg, uint16_t sendFlags)
{
return SendTo(addr, INET_NULL_INTERFACEID, std::move(msg), sendFlags);
}
Expand Down Expand Up @@ -547,7 +547,7 @@ INET_ERROR RawEndPoint::SendTo(const IPAddress & addr, chip::System::PacketBuffe
* @details
* Send the ICMP message in \c msg to the destination given in \c addr.
*/
INET_ERROR RawEndPoint::SendTo(const IPAddress & addr, InterfaceId intfId, chip::System::PacketBufferHandle msg, uint16_t sendFlags)
INET_ERROR RawEndPoint::SendTo(const IPAddress & addr, InterfaceId intfId, chip::System::PacketBufferHandle && msg, uint16_t sendFlags)
{
IPPacketInfo pktInfo;
pktInfo.Clear();
Expand Down Expand Up @@ -829,7 +829,7 @@ InterfaceId RawEndPoint::GetBoundInterface()

#if CHIP_SYSTEM_CONFIG_USE_LWIP

void RawEndPoint::HandleDataReceived(System::PacketBufferHandle msg)
void RawEndPoint::HandleDataReceived(System::PacketBufferHandle && msg)
{
IPEndPointBasis::HandleDataReceived(std::move(msg));
}
Expand Down
6 changes: 3 additions & 3 deletions src/inet/RawEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ class DLL_EXPORT RawEndPoint : public IPEndPointBasis
INET_ERROR BindInterface(IPAddressType addrType, InterfaceId intfId);
InterfaceId GetBoundInterface();
INET_ERROR Listen();
INET_ERROR SendTo(const IPAddress & addr, chip::System::PacketBufferHandle msg, uint16_t sendFlags = 0);
INET_ERROR SendTo(const IPAddress & addr, InterfaceId intfId, chip::System::PacketBufferHandle msg, uint16_t sendFlags = 0);
INET_ERROR SendTo(const IPAddress & addr, chip::System::PacketBufferHandle && msg, uint16_t sendFlags = 0);
INET_ERROR SendTo(const IPAddress & addr, InterfaceId intfId, chip::System::PacketBufferHandle && msg, uint16_t sendFlags = 0);
INET_ERROR SendMsg(const IPPacketInfo * pktInfo, chip::System::PacketBufferHandle msg, uint16_t sendFlags = 0);
INET_ERROR SetICMPFilter(uint8_t numICMPTypes, const uint8_t * aICMPTypes);
void Close();
Expand All @@ -95,7 +95,7 @@ class DLL_EXPORT RawEndPoint : public IPEndPointBasis
uint8_t NumICMPTypes;
const uint8_t * ICMPTypes;

void HandleDataReceived(chip::System::PacketBufferHandle msg);
void HandleDataReceived(chip::System::PacketBufferHandle && msg);
INET_ERROR GetPCB(IPAddressType addrType);

#if LWIP_VERSION_MAJOR > 1 || LWIP_VERSION_MINOR >= 5
Expand Down
6 changes: 3 additions & 3 deletions src/inet/UDPEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ void UDPEndPoint::Free()
/**
* A synonym for <tt>SendTo(addr, port, INET_NULL_INTERFACEID, msg, sendFlags)</tt>.
*/
INET_ERROR UDPEndPoint::SendTo(const IPAddress & addr, uint16_t port, chip::System::PacketBufferHandle msg, uint16_t sendFlags)
INET_ERROR UDPEndPoint::SendTo(const IPAddress & addr, uint16_t port, chip::System::PacketBufferHandle && msg, uint16_t sendFlags)
{
return SendTo(addr, port, INET_NULL_INTERFACEID, std::move(msg), sendFlags);
}
Expand Down Expand Up @@ -491,7 +491,7 @@ INET_ERROR UDPEndPoint::SendTo(const IPAddress & addr, uint16_t port, chip::Syst
* identifier for IPv6 link-local destinations) and \c port with the
* transmit option flags encoded in \c sendFlags.
*/
INET_ERROR UDPEndPoint::SendTo(const IPAddress & addr, uint16_t port, InterfaceId intfId, chip::System::PacketBufferHandle msg,
INET_ERROR UDPEndPoint::SendTo(const IPAddress & addr, uint16_t port, InterfaceId intfId, chip::System::PacketBufferHandle && msg,
uint16_t sendFlags)
{
IPPacketInfo pktInfo;
Expand Down Expand Up @@ -775,7 +775,7 @@ uint16_t UDPEndPoint::GetBoundPort()

#if CHIP_SYSTEM_CONFIG_USE_LWIP

void UDPEndPoint::HandleDataReceived(System::PacketBufferHandle msg)
void UDPEndPoint::HandleDataReceived(System::PacketBufferHandle && msg)
{
IPEndPointBasis::HandleDataReceived(std::move(msg));
}
Expand Down
6 changes: 3 additions & 3 deletions src/inet/UDPEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ class DLL_EXPORT UDPEndPoint : public IPEndPointBasis
InterfaceId GetBoundInterface();
uint16_t GetBoundPort();
INET_ERROR Listen();
INET_ERROR SendTo(const IPAddress & addr, uint16_t port, chip::System::PacketBufferHandle msg, uint16_t sendFlags = 0);
INET_ERROR SendTo(const IPAddress & addr, uint16_t port, InterfaceId intfId, chip::System::PacketBufferHandle msg,
INET_ERROR SendTo(const IPAddress & addr, uint16_t port, chip::System::PacketBufferHandle && msg, uint16_t sendFlags = 0);
INET_ERROR SendTo(const IPAddress & addr, uint16_t port, InterfaceId intfId, chip::System::PacketBufferHandle && msg,
uint16_t sendFlags = 0);
INET_ERROR SendMsg(const IPPacketInfo * pktInfo, chip::System::PacketBufferHandle msg, uint16_t sendFlags = 0);
void Close();
Expand All @@ -74,7 +74,7 @@ class DLL_EXPORT UDPEndPoint : public IPEndPointBasis
void Init(InetLayer * inetLayer);

#if CHIP_SYSTEM_CONFIG_USE_LWIP
void HandleDataReceived(chip::System::PacketBufferHandle msg);
void HandleDataReceived(chip::System::PacketBufferHandle && msg);
INET_ERROR GetPCB(IPAddressType addrType4);
#if LWIP_VERSION_MAJOR > 1 || LWIP_VERSION_MINOR >= 5
static void LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb, struct pbuf * p, const ip_addr_t * addr, u16_t port);
Expand Down
4 changes: 2 additions & 2 deletions src/inet/tests/TestInetLayerCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ static bool HandleICMPDataReceived(PacketBufferHandle aBuffer, uint16_t aHeaderL
return (lStatus);
}

bool HandleICMPv4DataReceived(PacketBufferHandle aBuffer, TransferStats & aStats, bool aStatsByPacket, bool aCheckBuffer)
bool HandleICMPv4DataReceived(PacketBufferHandle && aBuffer, TransferStats & aStats, bool aStatsByPacket, bool aCheckBuffer)
{
const uint16_t lICMPHeaderLength = sizeof(ICMPv4EchoHeader);
bool lStatus;
Expand All @@ -305,7 +305,7 @@ bool HandleICMPv4DataReceived(PacketBufferHandle aBuffer, TransferStats & aStats
return (lStatus);
}

bool HandleICMPv6DataReceived(PacketBufferHandle aBuffer, TransferStats & aStats, bool aStatsByPacket, bool aCheckBuffer)
bool HandleICMPv6DataReceived(PacketBufferHandle && aBuffer, TransferStats & aStats, bool aStatsByPacket, bool aCheckBuffer)
{
const uint16_t lICMPHeaderLength = sizeof(ICMPv6EchoHeader);
bool lStatus;
Expand Down
4 changes: 2 additions & 2 deletions src/inet/tests/TestInetLayerCommon.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ extern bool HandleDataReceived(const System::PacketBufferHandle & aBuffer, Trans
bool aCheckBuffer, uint8_t aFirstValue);
extern bool HandleDataReceived(const System::PacketBufferHandle & aBuffer, TransferStats & aStats, bool aStatsByPacket,
bool aCheckBuffer);
extern bool HandleICMPv4DataReceived(System::PacketBufferHandle aBuffer, TransferStats & aStats, bool aStatsByPacket,
extern bool HandleICMPv4DataReceived(System::PacketBufferHandle && aBuffer, TransferStats & aStats, bool aStatsByPacket,
bool aCheckBuffer);
extern bool HandleICMPv6DataReceived(System::PacketBufferHandle aBuffer, TransferStats & aStats, bool aStatsByPacket,
extern bool HandleICMPv6DataReceived(System::PacketBufferHandle && aBuffer, TransferStats & aStats, bool aStatsByPacket,
bool aCheckBuffer);

// Timer Callback Handler
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/echo/Echo.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class DLL_EXPORT EchoClient : public Messaging::ExchangeDelegate
* Other CHIP_ERROR codes as returned by the lower layers.
*
*/
CHIP_ERROR SendEchoRequest(System::PacketBufferHandle payload);
CHIP_ERROR SendEchoRequest(System::PacketBufferHandle && payload);

private:
Messaging::ExchangeManager * mExchangeMgr = nullptr;
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/echo/EchoClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void EchoClient::Shutdown()
}
}

CHIP_ERROR EchoClient::SendEchoRequest(System::PacketBufferHandle payload)
CHIP_ERROR EchoClient::SendEchoRequest(System::PacketBufferHandle && payload)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand Down
2 changes: 1 addition & 1 deletion src/system/SystemPacketBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ void PacketBuffer::AddToEnd_ForNow(PacketBuffer * aPacket)
#endif // !CHIP_SYSTEM_CONFIG_USE_LWIP
}

void PacketBuffer::AddToEnd(PacketBufferHandle aPacketHandle)
void PacketBuffer::AddToEnd(PacketBufferHandle && aPacketHandle)
{
AddToEnd_ForNow(aPacketHandle.Release_ForNow());
}
Expand Down
2 changes: 1 addition & 1 deletion src/system/SystemPacketBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class DLL_EXPORT PacketBuffer : private pbuf
*
* @param[in] aPacket - the packet buffer to be added to the end of the current chain.
*/
void AddToEnd(PacketBufferHandle aPacket);
void AddToEnd(PacketBufferHandle && aPacket);

/**
* Move data from subsequent buffers in the chain into the current buffer until it is full.
Expand Down
10 changes: 5 additions & 5 deletions src/system/TLVPacketBufferBackingStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore
{
public:
TLVPacketBufferBackingStore() : mHeadBuffer(nullptr), mCurrentBuffer(nullptr), mUseChainedBuffers(false) {}
TLVPacketBufferBackingStore(chip::System::PacketBufferHandle buffer, bool useChainedBuffers = false)
TLVPacketBufferBackingStore(chip::System::PacketBufferHandle && buffer, bool useChainedBuffers = false)
{
Init(std::move(buffer), useChainedBuffers);
}
Expand All @@ -55,13 +55,13 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore
*
* @note This must take place before initializing a TLV class with this backing store.
*/
void Init(chip::System::PacketBufferHandle buffer, bool useChainedBuffers = false)
void Init(chip::System::PacketBufferHandle && buffer, bool useChainedBuffers = false)
{
mHeadBuffer = std::move(buffer);
mCurrentBuffer = mHeadBuffer.Retain();
mUseChainedBuffers = useChainedBuffers;
}
void Adopt(chip::System::PacketBufferHandle buffer) { Init(std::move(buffer), mUseChainedBuffers); }
void Adopt(chip::System::PacketBufferHandle && buffer) { Init(std::move(buffer), mUseChainedBuffers); }

/**
* Release ownership of the backing packet buffer.
Expand Down Expand Up @@ -98,7 +98,7 @@ class DLL_EXPORT PacketBufferTLVReader : public chip::TLV::TLVReader
* If true, advance to the next buffer in the chain once all data
* in the current buffer has been consumed.
*/
void Init(chip::System::PacketBufferHandle buffer, bool useChainedBuffers = false)
void Init(chip::System::PacketBufferHandle && buffer, bool useChainedBuffers = false)
{
mBackingStore.Init(std::move(buffer), useChainedBuffers);
chip::TLV::TLVReader::Init(mBackingStore);
Expand All @@ -120,7 +120,7 @@ class DLL_EXPORT PacketBufferTLVWriter : public chip::TLV::TLVWriter
* in the current buffer has been consumed. Once all existing buffers
* have been used, new PacketBuffers will be allocated as necessary.
*/
void Init(chip::System::PacketBufferHandle buffer, bool useChainedBuffers = false)
void Init(chip::System::PacketBufferHandle && buffer, bool useChainedBuffers = false)
{
mBackingStore.Init(std::move(buffer), useChainedBuffers);
chip::TLV::TLVWriter::Init(mBackingStore);
Expand Down
4 changes: 2 additions & 2 deletions src/transport/SecureSessionMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,14 @@ Transport::Type SecureSessionMgr::GetTransportType(NodeId peerNodeId)
return Transport::Type::kUndefined;
}

CHIP_ERROR SecureSessionMgr::SendMessage(SecureSessionHandle session, System::PacketBufferHandle msgBuf)
CHIP_ERROR SecureSessionMgr::SendMessage(SecureSessionHandle session, System::PacketBufferHandle && msgBuf)
{
PayloadHeader unusedPayloadHeader;
return SendMessage(session, unusedPayloadHeader, std::move(msgBuf));
}

CHIP_ERROR SecureSessionMgr::SendMessage(SecureSessionHandle session, PayloadHeader & payloadHeader,
System::PacketBufferHandle msgBuf, EncryptedPacketBufferHandle * bufferRetainSlot)
System::PacketBufferHandle && msgBuf, EncryptedPacketBufferHandle * bufferRetainSlot)
{
PacketHeader ununsedPacketHeader;
return SendMessage(session, payloadHeader, ununsedPacketHeader, std::move(msgBuf), bufferRetainSlot,
Expand Down
4 changes: 2 additions & 2 deletions src/transport/SecureSessionMgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ class DLL_EXPORT SecureSessionMgr : public TransportMgrDelegate
* returns success, the encrypted data that was sent, as well as various other information needed
* to retransmit it, will be stored in *bufferRetainSlot.
*/
CHIP_ERROR SendMessage(SecureSessionHandle session, System::PacketBufferHandle msgBuf);
CHIP_ERROR SendMessage(SecureSessionHandle session, PayloadHeader & payloadHeader, System::PacketBufferHandle msgBuf,
CHIP_ERROR SendMessage(SecureSessionHandle session, System::PacketBufferHandle && msgBuf);
CHIP_ERROR SendMessage(SecureSessionHandle session, PayloadHeader & payloadHeader, System::PacketBufferHandle && msgBuf,
EncryptedPacketBufferHandle * bufferRetainSlot = nullptr);
CHIP_ERROR SendEncryptedMessage(SecureSessionHandle session, EncryptedPacketBufferHandle msgBuf,
EncryptedPacketBufferHandle * bufferRetainSlot);
Expand Down
2 changes: 1 addition & 1 deletion src/transport/TransportMgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class TransportMgrBase
public:
CHIP_ERROR Init(Transport::Base * transport);

CHIP_ERROR SendMessage(const PacketHeader & header, const Transport::PeerAddress & address, System::PacketBufferHandle msgBuf)
CHIP_ERROR SendMessage(const PacketHeader & header, const Transport::PeerAddress & address, System::PacketBufferHandle && msgBuf)
{
return mTransport->SendMessage(header, address, std::move(msgBuf));
}
Expand Down
2 changes: 1 addition & 1 deletion src/transport/raw/Tuple.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class Tuple : public Base
* @param msgBuf the data to send.
*/
template <size_t N, typename std::enable_if<(N < sizeof...(TransportTypes))>::type * = nullptr>
CHIP_ERROR SendMessageImpl(const PacketHeader & header, const PeerAddress & address, System::PacketBufferHandle msgBuf)
CHIP_ERROR SendMessageImpl(const PacketHeader & header, const PeerAddress & address, System::PacketBufferHandle && msgBuf)
{
Base * base = &std::get<N>(mTransports);
if (base->CanSendToPeer(address))
Expand Down

0 comments on commit 21318aa

Please sign in to comment.