From 922d6413d539ed8bd3721655ab9be87bc18c59f6 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Jul 2015 11:21:27 -0700 Subject: [PATCH 1/6] change some const unique_ptr to const NLPacket --- assignment-client/src/octree/OctreeQueryNode.cpp | 2 +- assignment-client/src/octree/OctreeQueryNode.h | 2 +- libraries/networking/src/SentPacketHistory.cpp | 6 +++--- libraries/networking/src/SentPacketHistory.h | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/assignment-client/src/octree/OctreeQueryNode.cpp b/assignment-client/src/octree/OctreeQueryNode.cpp index 43d6ea9ba6..173c1dd840 100644 --- a/assignment-client/src/octree/OctreeQueryNode.cpp +++ b/assignment-client/src/octree/OctreeQueryNode.cpp @@ -356,7 +356,7 @@ void OctreeQueryNode::dumpOutOfView() { } } -void OctreeQueryNode::packetSent(const std::unique_ptr& packet) { +void OctreeQueryNode::packetSent(const NLPacket& packet) { _sentPacketHistory.packetSent(_sequenceNumber, packet); _sequenceNumber++; } diff --git a/assignment-client/src/octree/OctreeQueryNode.h b/assignment-client/src/octree/OctreeQueryNode.h index 3dfe2ad931..045600fd98 100644 --- a/assignment-client/src/octree/OctreeQueryNode.h +++ b/assignment-client/src/octree/OctreeQueryNode.h @@ -105,7 +105,7 @@ public: bool isShuttingDown() const { return _isShuttingDown; } void octreePacketSent() { packetSent(_octreePacket); } - void packetSent(const std::unique_ptr& packet); + void packetSent(const NLPacket& packet); OCTREE_PACKET_SEQUENCE getSequenceNumber() const { return _sequenceNumber; } diff --git a/libraries/networking/src/SentPacketHistory.cpp b/libraries/networking/src/SentPacketHistory.cpp index 770973f8d8..4717970c2a 100644 --- a/libraries/networking/src/SentPacketHistory.cpp +++ b/libraries/networking/src/SentPacketHistory.cpp @@ -24,7 +24,7 @@ SentPacketHistory::SentPacketHistory(int size) } -void SentPacketHistory::packetSent(uint16_t sequenceNumber, const std::unique_ptr& packet) { +void SentPacketHistory::packetSent(uint16_t sequenceNumber, const NLPacket& packet) { // check if given seq number has the expected value. if not, something's wrong with // the code calling this function @@ -38,7 +38,7 @@ void SentPacketHistory::packetSent(uint16_t sequenceNumber, const std::unique_pt _sentPackets.insert(NLPacket::createCopy(packet)); } -const std::unique_ptr& SentPacketHistory::getPacket(uint16_t sequenceNumber) const { +const NLPacket& SentPacketHistory::getPacket(uint16_t sequenceNumber) const { const int UINT16_RANGE = std::numeric_limits::max() + 1; @@ -49,5 +49,5 @@ const std::unique_ptr& SentPacketHistory::getPacket(uint16_t sequenceN seqDiff += UINT16_RANGE; } - return *_sentPackets.get(seqDiff); + return *_sentPackets.get(seqDiff).get(); } diff --git a/libraries/networking/src/SentPacketHistory.h b/libraries/networking/src/SentPacketHistory.h index ad03f87f88..e9ba58d4ef 100644 --- a/libraries/networking/src/SentPacketHistory.h +++ b/libraries/networking/src/SentPacketHistory.h @@ -24,8 +24,8 @@ class SentPacketHistory { public: SentPacketHistory(int size = MAX_REASONABLE_SEQUENCE_GAP); - void packetSent(uint16_t sequenceNumber, const std::unique_ptr& packet); - const std::unique_ptr& getPacket(uint16_t sequenceNumber) const; + void packetSent(uint16_t sequenceNumber, const NLPacket& packet); + const NLPacket& getPacket(uint16_t sequenceNumber) const; private: RingBufferHistory> _sentPackets; // circular buffer From bd9b42bd4cbb4fc4f8ff48d65c41a7875274227b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Jul 2015 11:23:44 -0700 Subject: [PATCH 2/6] change createCopy in packet to take const & --- libraries/networking/src/NLPacket.cpp | 9 ++++----- libraries/networking/src/NLPacket.h | 2 +- libraries/networking/src/Packet.cpp | 5 ++--- libraries/networking/src/Packet.h | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index f61b47b4b2..f125cdc6b7 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -33,13 +33,12 @@ std::unique_ptr NLPacket::create(PacketType::Value type, int64_t size) if (size > maxPayloadSize(type)) { return std::unique_ptr(); } - + return std::unique_ptr(new NLPacket(type, size)); } -std::unique_ptr NLPacket::createCopy(const std::unique_ptr& other) { - Q_ASSERT(other); - return std::unique_ptr(new NLPacket(*other)); +std::unique_ptr NLPacket::createCopy(const NLPacket& other) { + return std::unique_ptr(new NLPacket(other)); } NLPacket::NLPacket(PacketType::Value type, int64_t size) : Packet(type, localHeaderSize(type) + size) { @@ -59,4 +58,4 @@ void NLPacket::setConnectionUuid(QUuid connectionUuid) { auto offset = Packet::totalHeadersSize() + ((NON_SOURCED_PACKETS.contains(_type)) ? 0 : NUM_BYTES_RFC4122_UUID); memcpy(_packet.get() + offset, connectionUuid.toRfc4122().constData(), NUM_BYTES_RFC4122_UUID); -} \ No newline at end of file +} diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index 744056c103..7ea449815b 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -19,7 +19,7 @@ class NLPacket : public Packet { public: static std::unique_ptr create(PacketType::Value type, int64_t size = -1); // Provided for convenience, try to limit use - static std::unique_ptr createCopy(const std::unique_ptr& other); + static std::unique_ptr createCopy(const NLPacket& other); static int64_t localHeaderSize(PacketType::Value type); static int64_t maxPayloadSize(PacketType::Value type); diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index aa0b407398..6d585ac4aa 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -38,9 +38,8 @@ std::unique_ptr Packet::create(PacketType::Value type, qint64 size) { return std::unique_ptr(new Packet(type, size)); } -std::unique_ptr Packet::createCopy(const std::unique_ptr& other) { - Q_ASSERT(other); - return std::unique_ptr(new Packet(*other)); +std::unique_ptr Packet::createCopy(const Packet& other) { + return std::unique_ptr(new Packet(other)); } qint64 Packet::totalHeadersSize() const { diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index 7bd8d81194..1de17602da 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -25,7 +25,7 @@ public: static std::unique_ptr create(PacketType::Value type, int64_t size = -1); // Provided for convenience, try to limit use - static std::unique_ptr createCopy(const std::unique_ptr& other); + static std::unique_ptr createCopy(const Packet& other); static qint64 localHeaderSize(PacketType::Value type); static qint64 maxPayloadSize(PacketType::Value type); From 6d2fea442616c2d311e6a6e60ad109323818a27b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Jul 2015 11:24:58 -0700 Subject: [PATCH 3/6] make NLPacket copy take const, fix get in SPH --- libraries/networking/src/NLPacket.cpp | 2 +- libraries/networking/src/NLPacket.h | 2 +- libraries/networking/src/SentPacketHistory.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index f125cdc6b7..4f17d3d754 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -44,7 +44,7 @@ std::unique_ptr NLPacket::createCopy(const NLPacket& other) { NLPacket::NLPacket(PacketType::Value type, int64_t size) : Packet(type, localHeaderSize(type) + size) { } -NLPacket::NLPacket(NLPacket& other) : Packet(other) { +NLPacket::NLPacket(const NLPacket& other) : Packet(other) { } void NLPacket::setSourceUuid(QUuid sourceUuid) { diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index 7ea449815b..9368b25720 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -29,7 +29,7 @@ public: protected: NLPacket(PacketType::Value type, int64_t size); - NLPacket(NLPacket& other); + NLPacket(const NLPacket& other); void setSourceUuid(QUuid sourceUuid); void setConnectionUuid(QUuid connectionUuid); diff --git a/libraries/networking/src/SentPacketHistory.cpp b/libraries/networking/src/SentPacketHistory.cpp index 4717970c2a..68bd5f1c67 100644 --- a/libraries/networking/src/SentPacketHistory.cpp +++ b/libraries/networking/src/SentPacketHistory.cpp @@ -49,5 +49,5 @@ const NLPacket& SentPacketHistory::getPacket(uint16_t sequenceNumber) const { seqDiff += UINT16_RANGE; } - return *_sentPackets.get(seqDiff).get(); + return *_sentPackets.get(seqDiff)->get(); } From 765265abd8b59630e47db0af9126601278d7db07 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Jul 2015 11:27:06 -0700 Subject: [PATCH 4/6] fix networking API calls in IceServer --- ice-server/src/IceServer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index bc76d818c3..24199b8bb8 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -56,12 +56,12 @@ void IceServer::processDatagrams() { PacketType::Value packetType = packetTypeForPacket(incomingPacket); - if (packetType == PacketType::IceServerHeartbeat) { + if (packetType == PacketType::ICEServerHeartbeat) { SharedNetworkPeer peer = addOrUpdateHeartbeatingPeer(incomingPacket); // so that we can send packets to the heartbeating peer when we need, we need to activate a socket now peer->activateMatchingOrNewSymmetricSocket(sendingSockAddr); - } else if (packetType == PacketType::IceServerQuery) { + } else if (packetType == PacketType::ICEServerQuery) { QDataStream heartbeatStream(incomingPacket); // this is a node hoping to connect to a heartbeating peer - do we have the heartbeating peer? @@ -128,13 +128,13 @@ SharedNetworkPeer IceServer::addOrUpdateHeartbeatingPeer(const QByteArray& incom } void IceServer::sendPeerInformationPacket(const NetworkPeer& peer, const HifiSockAddr* destinationSockAddr) { - auto peerPacket = Packet::create(PacketType::IceServerPeerInformation); + auto peerPacket = Packet::create(PacketType::ICEServerPeerInformation); // get the byte array for this peer peerPacket->write(peer.toByteArray()); // write the current packet - _serverSocket.writeDatagram(peerPacket.constData(), peerPacket.sizeUsed(), + _serverSocket.writeDatagram(peerPacket->getData(), peerPacket->getSizeWithHeader(), destinationSockAddr->getAddress(), destinationSockAddr->getPort()); } From c903d284bb66c6e60d0c457077b003cdd816ee78 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Jul 2015 11:31:57 -0700 Subject: [PATCH 5/6] fix for arguments that are now const NLPacket& --- libraries/networking/src/SentPacketHistory.cpp | 10 +++++++--- libraries/networking/src/SentPacketHistory.h | 2 +- libraries/octree/src/OctreeEditPacketSender.cpp | 8 ++++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/libraries/networking/src/SentPacketHistory.cpp b/libraries/networking/src/SentPacketHistory.cpp index 68bd5f1c67..bea144e6aa 100644 --- a/libraries/networking/src/SentPacketHistory.cpp +++ b/libraries/networking/src/SentPacketHistory.cpp @@ -38,7 +38,7 @@ void SentPacketHistory::packetSent(uint16_t sequenceNumber, const NLPacket& pack _sentPackets.insert(NLPacket::createCopy(packet)); } -const NLPacket& SentPacketHistory::getPacket(uint16_t sequenceNumber) const { +const NLPacket* SentPacketHistory::getPacket(uint16_t sequenceNumber) const { const int UINT16_RANGE = std::numeric_limits::max() + 1; @@ -48,6 +48,10 @@ const NLPacket& SentPacketHistory::getPacket(uint16_t sequenceNumber) const { if (seqDiff < 0) { seqDiff += UINT16_RANGE; } - - return *_sentPackets.get(seqDiff)->get(); + auto packet = _sentPackets.get(seqDiff); + if (packet) { + return packet->get(); + } else { + return nullptr; + } } diff --git a/libraries/networking/src/SentPacketHistory.h b/libraries/networking/src/SentPacketHistory.h index e9ba58d4ef..2971df12f5 100644 --- a/libraries/networking/src/SentPacketHistory.h +++ b/libraries/networking/src/SentPacketHistory.h @@ -25,7 +25,7 @@ public: SentPacketHistory(int size = MAX_REASONABLE_SEQUENCE_GAP); void packetSent(uint16_t sequenceNumber, const NLPacket& packet); - const NLPacket& getPacket(uint16_t sequenceNumber) const; + const NLPacket* getPacket(uint16_t sequenceNumber) const; private: RingBufferHistory> _sentPackets; // circular buffer diff --git a/libraries/octree/src/OctreeEditPacketSender.cpp b/libraries/octree/src/OctreeEditPacketSender.cpp index 86d25af759..4151f19821 100644 --- a/libraries/octree/src/OctreeEditPacketSender.cpp +++ b/libraries/octree/src/OctreeEditPacketSender.cpp @@ -108,7 +108,7 @@ void OctreeEditPacketSender::queuePacketToNode(const QUuid& nodeUUID, std::uniqu } // add packet to history - _sentPacketHistories[nodeUUID].packetSent(sequence, packet); + _sentPacketHistories[nodeUUID].packetSent(sequence, *packet.get()); queuePacketForSending(node, std::move(packet)); } @@ -186,7 +186,7 @@ void OctreeEditPacketSender::queuePacketToNodes(std::unique_ptr packet if (isMyJurisdiction) { // make a copy of this packet for this node and queue - auto packetCopy = NLPacket::createCopy(packet); + auto packetCopy = NLPacket::createCopy(*packet.get()); queuePacketToNode(nodeUUID, std::move(packetCopy)); } } @@ -362,10 +362,10 @@ void OctreeEditPacketSender::processNackPacket(const QByteArray& packet) { dataAt += sizeof(unsigned short int); // retrieve packet from history - const std::unique_ptr& packet = sentPacketHistory.getPacket(sequenceNumber); + const NLPacket* packet = sentPacketHistory.getPacket(sequenceNumber); if (packet) { const SharedNodePointer& node = DependencyManager::get()->nodeWithUUID(sendingNodeUUID); - queuePacketForSending(node, NLPacket::createCopy(packet)); + queuePacketForSending(node, NLPacket::createCopy(*packet)); } } } From 7e07754773c860be329ff1f053bbd0a0635d7e7e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Jul 2015 11:35:12 -0700 Subject: [PATCH 6/6] fix octreePacketSent call with const NLPacket& --- assignment-client/src/octree/OctreeQueryNode.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/octree/OctreeQueryNode.h b/assignment-client/src/octree/OctreeQueryNode.h index 0329c7756a..cdd9270014 100644 --- a/assignment-client/src/octree/OctreeQueryNode.h +++ b/assignment-client/src/octree/OctreeQueryNode.h @@ -103,7 +103,7 @@ public: void forceNodeShutdown(); bool isShuttingDown() const { return _isShuttingDown; } - void octreePacketSent() { packetSent(_octreePacket); } + void octreePacketSent() { packetSent(*_octreePacket.get()); } void packetSent(const NLPacket& packet); OCTREE_PACKET_SEQUENCE getSequenceNumber() const { return _sequenceNumber; }