From 11c58ebc9ec98e9b4a81c4eb81bd1adb6ccff5ff Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 7 Jul 2015 15:07:14 -0700 Subject: [PATCH 1/3] Added Packet::createCopy and getData --- libraries/networking/src/Packet.cpp | 18 +++++++----------- libraries/networking/src/Packet.h | 28 +++++++++++++++------------- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index ca49100efe..6a02af803b 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -38,6 +38,12 @@ 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)); +} + qint64 Packet::totalHeadersSize() const { return localHeaderSize(); } @@ -74,7 +80,6 @@ Packet& Packet::operator=(const Packet& other) { memcpy(_packet.get(), other._packet.get(), _packetSize); _payloadStart = _packet.get() + (other._payloadStart - other._packet.get()); - _position = other._position; _capacity = other._capacity; _sizeUsed = other._sizeUsed; @@ -91,7 +96,6 @@ Packet& Packet::operator=(Packet&& other) { _packet = std::move(other._packet); _payloadStart = other._payloadStart; - _position = other._position; _capacity = other._capacity; _sizeUsed = other._sizeUsed; @@ -132,7 +136,7 @@ void Packet::setPacketTypeAndVersion(PacketType::Value type) { auto offset = packArithmeticallyCodedValue(type, _packet.get()); // Pack the packet version - auto version { versionForPacketType(type) }; + auto version = versionForPacketType(type); memcpy(_packet.get() + offset, &version, sizeof(version)); } @@ -144,14 +148,6 @@ void Packet::setSequenceNumber(SequenceNumber seqNum) { &seqNum, sizeof(seqNum)); } -bool Packet::seek(qint64 pos) { - bool valid = (pos >= 0) && (pos < size()); - if (valid) { - _position = pos; - } - return valid; -} - static const qint64 PACKET_WRITE_ERROR = -1; qint64 Packet::writeData(const char* data, qint64 maxSize) { // make sure we have the space required to write this block diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index 6177cbd60e..63f6db8fda 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -23,6 +23,8 @@ public: using SequenceNumber = uint16_t; 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 qint64 localHeaderSize(PacketType::Value type); static qint64 maxPayloadSize(PacketType::Value type); @@ -30,10 +32,14 @@ public: virtual qint64 totalHeadersSize() const; // Cumulated size of all the headers virtual qint64 localHeaderSize() const; // Current level's header size - // Payload direct access, use responsibly! + // Payload direct access to the payload, use responsibly! char* getPayload() { return _payloadStart; } const char* getPayload() const { return _payloadStart; } + // Return direct access to the entire packet, use responsibly! + char* getData() { return _packet.get(); } + const char* getData() const { return _packet.get(); } + qint64 getSizeWithHeader() const { return localHeaderSize() + getSizeUsed(); } qint64 getSizeUsed() const { return _sizeUsed; } void setSizeUsed(qint64 sizeUsed) { _sizeUsed = sizeUsed; } @@ -46,21 +52,12 @@ public: // QIODevice virtual functions // WARNING: Those methods all refer to the payload ONLY and NOT the entire packet - virtual bool atEnd() const { return _position == _capacity; } - virtual qint64 bytesAvailable() const { return size() - pos(); } - virtual bool canReadLine() const { return false; } virtual bool isSequential() const { return false; } - virtual qint64 pos() const { return _position; } - virtual bool reset() { return seek(0); } - virtual bool seek(qint64 pos); + virtual bool reset() { setSizeUsed(0); return QIODevice::reset(); } virtual qint64 size() const { return _capacity; } protected: Packet(PacketType::Value type, int64_t size); - Packet(const Packet& other); - Packet& operator=(const Packet& other); - Packet(Packet&& other); - Packet& operator=(Packet&& other); // QIODevice virtual functions virtual qint64 writeData(const char* data, qint64 maxSize); @@ -70,16 +67,21 @@ protected: void setPacketTypeAndVersion(PacketType::Value type); void setSequenceNumber(SequenceNumber seqNum); - PacketType::Value _type; + PacketType::Value _type; // Packet type qint64 _packetSize = 0; // Total size of the allocated memory std::unique_ptr _packet; // Allocated memory char* _payloadStart = nullptr; // Start of the payload - qint64 _position = 0; // Current position in the payload qint64 _capacity = 0; // Total capacity of the payload qint64 _sizeUsed = 0; // How much of the payload is actually used + +private: + Packet(const Packet& other); + Packet& operator=(const Packet& other); + Packet(Packet&& other); + Packet& operator=(Packet&& other); }; #endif // hifi_Packet_h \ No newline at end of file From b4ab6828bbae18cd5fd752d0f11a72882e99d3b4 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 7 Jul 2015 15:43:16 -0700 Subject: [PATCH 2/3] Added setPacketType to Packet class --- libraries/networking/src/Packet.cpp | 25 +++++++++++++++++-------- libraries/networking/src/Packet.h | 15 +++++++++------ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index 6a02af803b..65d0899a94 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -62,11 +62,11 @@ Packet::Packet(PacketType::Value type, qint64 size) : Q_ASSERT(size <= maxPayloadSize(type)); // copy packet type and version in header - setPacketTypeAndVersion(type); + writePacketTypeAndVersion(type); // Set control bit and sequence number to 0 if necessary if (SEQUENCE_NUMBERED_PACKETS.contains(type)) { - setSequenceNumber(0); + writeSequenceNumber(0); } } @@ -103,15 +103,24 @@ Packet& Packet::operator=(Packet&& other) { return *this; } -PacketType::Value Packet::getPacketType() const { +void Packet::setPacketType(PacketType::Value type) { + auto currentHeaderSize = totalHeadersSize(); + _type = type; + writePacketTypeAndVersion(_type); + + // Setting new packet type with a different header size not currently supported + Q_ASSERT(currentHeaderSize == totalHeadersSize()); +} + +PacketType::Value Packet::readPacketType() const { return (PacketType::Value)arithmeticCodingValueFromBuffer(_packet.get()); } -PacketVersion Packet::getPacketTypeVersion() const { +PacketVersion Packet::readPacketTypeVersion() const { return *reinterpret_cast(_packet.get() + numBytesForArithmeticCodedPacketType(_type)); } -Packet::SequenceNumber Packet::getSequenceNumber() const { +Packet::SequenceNumber Packet::readSequenceNumber() const { if (SEQUENCE_NUMBERED_PACKETS.contains(_type)) { SequenceNumber seqNum = *reinterpret_cast(_packet.get() + numBytesForArithmeticCodedPacketType(_type) + @@ -121,7 +130,7 @@ Packet::SequenceNumber Packet::getSequenceNumber() const { return -1; } -bool Packet::isControlPacket() const { +bool Packet::readIsControlPacket() const { if (SEQUENCE_NUMBERED_PACKETS.contains(_type)) { SequenceNumber seqNum = *reinterpret_cast(_packet.get() + numBytesForArithmeticCodedPacketType(_type) + @@ -131,7 +140,7 @@ bool Packet::isControlPacket() const { return false; } -void Packet::setPacketTypeAndVersion(PacketType::Value type) { +void Packet::writePacketTypeAndVersion(PacketType::Value type) { // Pack the packet type auto offset = packArithmeticallyCodedValue(type, _packet.get()); @@ -140,7 +149,7 @@ void Packet::setPacketTypeAndVersion(PacketType::Value type) { memcpy(_packet.get() + offset, &version, sizeof(version)); } -void Packet::setSequenceNumber(SequenceNumber seqNum) { +void Packet::writeSequenceNumber(SequenceNumber seqNum) { // Here we are overriding the control bit to 0. // But that is not an issue since we should only ever set the seqNum // for data packets going out diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index 63f6db8fda..9d58d1e2e1 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -40,15 +40,18 @@ public: char* getData() { return _packet.get(); } const char* getData() const { return _packet.get(); } + PacketType::Value getPacketType() const { return _type; } + void setPacketType(PacketType::Value type); + qint64 getSizeWithHeader() const { return localHeaderSize() + getSizeUsed(); } qint64 getSizeUsed() const { return _sizeUsed; } void setSizeUsed(qint64 sizeUsed) { _sizeUsed = sizeUsed; } // Header readers - PacketType::Value getPacketType() const; - PacketVersion getPacketTypeVersion() const; - SequenceNumber getSequenceNumber() const; - bool isControlPacket() const; + PacketType::Value readPacketType() const; + PacketVersion readPacketTypeVersion() const; + SequenceNumber readSequenceNumber() const; + bool readIsControlPacket() const; // QIODevice virtual functions // WARNING: Those methods all refer to the payload ONLY and NOT the entire packet @@ -64,8 +67,8 @@ protected: virtual qint64 readData(char* data, qint64 maxSize); // Header writers - void setPacketTypeAndVersion(PacketType::Value type); - void setSequenceNumber(SequenceNumber seqNum); + void writePacketTypeAndVersion(PacketType::Value type); + void writeSequenceNumber(SequenceNumber seqNum); PacketType::Value _type; // Packet type From d496b33ae6683d25ef12f9e82ee95db30aa26d57 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 7 Jul 2015 15:51:29 -0700 Subject: [PATCH 3/3] Compile fixes --- libraries/networking/src/LimitedNodeList.h | 8 +++++--- libraries/networking/src/NetworkPacket.h | 2 +- libraries/networking/src/PacketList.h | 12 +++++++----- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index ceb0427ef0..fdeaefb80a 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -38,6 +38,7 @@ #include "Node.h" #include "NLPacket.h" #include "PacketHeaders.h" +#include "PacketList.h" #include "UUIDHasher.h" const int MAX_PACKET_SIZE = 1450; @@ -67,6 +68,7 @@ Q_DECLARE_METATYPE(SharedNodePointer) using namespace tbb; typedef std::pair UUIDNodePair; typedef concurrent_unordered_map NodeHash; +using NLPacketList = PacketList; typedef quint8 PingType_t; namespace PingType { @@ -147,8 +149,8 @@ public: qint64 sendUnreliablePacket(NLPacket& packet, const HifiSockAddr& sockAddr) {}; qint64 sendPacket(NLPacket&& packet, const SharedNodePointer& destinationNode) {}; qint64 sendPacket(NLPacket&& packet, const HifiSockAddr& sockAddr) {}; - qint64 sendPacketList(PacketList& packetList, const SharedNodePointer& destinationNode) {}; - qint64 sendPacketList(PacketList& packetList, const HifiSockAddr& sockAddr) {}; + qint64 sendPacketList(NLPacketList& packetList, const SharedNodePointer& destinationNode) {}; + qint64 sendPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr) {}; void (*linkedDataCreateCallback)(Node *); @@ -173,7 +175,7 @@ public: int updateNodeWithDataFromPacket(const SharedNodePointer& matchingNode, const QByteArray& packet); int findNodeAndUpdateWithDataFromPacket(const QByteArray& packet); - unsigned broadcastToNodes(PacketList& packetList, const NodeSet& destinationNodeTypes) {}; + unsigned broadcastToNodes(NLPacketList& packetList, const NodeSet& destinationNodeTypes) {}; SharedNodePointer soloNodeOfType(char nodeType); void getPacketStats(float &packetsPerSecond, float &bytesPerSecond); diff --git a/libraries/networking/src/NetworkPacket.h b/libraries/networking/src/NetworkPacket.h index c42ad286ae..a4484f31d5 100644 --- a/libraries/networking/src/NetworkPacket.h +++ b/libraries/networking/src/NetworkPacket.h @@ -19,7 +19,7 @@ /// Storage of not-yet processed inbound, or not yet sent outbound generic UDP network packet class NetworkPacket { public: - NetworkPacket() { } + NetworkPacket(); NetworkPacket(const NetworkPacket& packet); // copy constructor NetworkPacket& operator= (const NetworkPacket& other); // copy assignment diff --git a/libraries/networking/src/PacketList.h b/libraries/networking/src/PacketList.h index 1b0905a1c8..86df44a968 100644 --- a/libraries/networking/src/PacketList.h +++ b/libraries/networking/src/PacketList.h @@ -12,7 +12,9 @@ #ifndef hifi_PacketList_h #define hifi_PacketList_h -#pragma once +#include + +#include "PacketHeaders.h" template class PacketList : public QIODevice { public: @@ -20,7 +22,7 @@ public: virtual bool isSequential() const { return true; } - void startSegment() { _segmentStartIndex = currentPacket->payload().pos(); } + void startSegment() { _segmentStartIndex = _currentPacket->payload().pos(); } void endSegment() { _segmentStartIndex = -1; } void closeCurrentPacket(); @@ -28,7 +30,7 @@ public: void setExtendedHeader(const QByteArray& extendedHeader) { _extendedHeader = extendedHeader; } protected: qint64 writeData(const char* data, qint64 maxSize); - qint64 readData(const char* data, qint64 maxSize) { return 0 }; + qint64 readData(const char* data, qint64 maxSize) { return 0; }; private: void createPacketWithExtendedHeader(); @@ -40,7 +42,7 @@ private: int _segmentStartIndex = -1; - QByteArray _extendedHeader = extendedHeader; -} + QByteArray _extendedHeader; +}; #endif // hifi_PacketList_h