From 7c87ee3a725c93dfc6484583854fc9610c8d0aa7 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 29 Jul 2015 17:37:33 -0700 Subject: [PATCH] type and version fixes after refactor --- libraries/networking/src/NLPacket.cpp | 12 ++++++------ libraries/networking/src/NodeList.cpp | 1 + libraries/networking/src/udt/BasePacket.cpp | 3 +-- libraries/networking/src/udt/BasePacket.h | 2 +- libraries/networking/src/udt/Constants.h | 3 ++- libraries/networking/src/udt/ControlPacket.cpp | 8 +++++--- libraries/networking/src/udt/Packet.cpp | 13 +++++++------ libraries/networking/src/udt/Socket.cpp | 3 +++ 8 files changed, 26 insertions(+), 19 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 5359d6cf2c..4568561fa2 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -26,7 +26,7 @@ qint64 NLPacket::maxPayloadSize() const { } qint64 NLPacket::totalHeadersSize() const { - return localHeaderSize() + Packet::localHeaderSize(); + return Packet::totalHeadersSize() + localHeaderSize(); } qint64 NLPacket::localHeaderSize() const { @@ -84,7 +84,7 @@ NLPacket::NLPacket(PacketType type, bool isReliable, bool isPartOfMessage) : _type(type), _version(versionForPacketType(type)) { - adjustPayloadStartAndCapacity(); + adjustPayloadStartAndCapacity(localHeaderSize()); writeTypeAndVersion(); } @@ -96,7 +96,7 @@ NLPacket::NLPacket(PacketType type, qint64 size, bool isReliable, bool isPartOfM { Q_ASSERT(size >= 0); - adjustPayloadStartAndCapacity(); + adjustPayloadStartAndCapacity(localHeaderSize()); writeTypeAndVersion(); } @@ -108,7 +108,7 @@ NLPacket::NLPacket(std::unique_ptr packet) : readVersion(); readSourceID(); - adjustPayloadStartAndCapacity(_payloadSize > 0); + adjustPayloadStartAndCapacity(localHeaderSize(), _payloadSize > 0); } NLPacket::NLPacket(const NLPacket& other) : Packet(other) { @@ -123,7 +123,7 @@ NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& // sanity check before we decrease the payloadSize with the payloadCapacity Q_ASSERT(_payloadSize == _payloadCapacity); - adjustPayloadStartAndCapacity(_payloadSize > 0); + adjustPayloadStartAndCapacity(localHeaderSize(), _payloadSize > 0); readType(); readVersion(); @@ -194,7 +194,7 @@ QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUu } void NLPacket::writeTypeAndVersion() { - auto headerOffset = Packet::totalHeadersSize(); + auto headerOffset = Packet::localHeaderSize(); // Pack the packet type memcpy(_packet.get() + headerOffset, &_type, sizeof(PacketType)); diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 0864cb9fcd..d55489a84d 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -252,6 +252,7 @@ void NodeList::sendDomainServerCheckIn() { } auto domainPacket = NLPacket::create(domainPacketType); + QDataStream packetStream(domainPacket.get()); if (domainPacketType == PacketType::DomainConnectRequest) { diff --git a/libraries/networking/src/udt/BasePacket.cpp b/libraries/networking/src/udt/BasePacket.cpp index 083a62b155..38fb9e849e 100644 --- a/libraries/networking/src/udt/BasePacket.cpp +++ b/libraries/networking/src/udt/BasePacket.cpp @@ -177,8 +177,7 @@ qint64 BasePacket::readData(char* dest, qint64 maxSize) { return numBytesToRead; } -void BasePacket::adjustPayloadStartAndCapacity(bool shouldDecreasePayloadSize) { - qint64 headerSize = localHeaderSize(); +void BasePacket::adjustPayloadStartAndCapacity(qint64 headerSize, bool shouldDecreasePayloadSize) { _payloadStart += headerSize; _payloadCapacity -= headerSize; diff --git a/libraries/networking/src/udt/BasePacket.h b/libraries/networking/src/udt/BasePacket.h index 7ebf2d1e65..d8c1aaeddf 100644 --- a/libraries/networking/src/udt/BasePacket.h +++ b/libraries/networking/src/udt/BasePacket.h @@ -86,7 +86,7 @@ protected: virtual qint64 writeData(const char* data, qint64 maxSize); virtual qint64 readData(char* data, qint64 maxSize); - virtual void adjustPayloadStartAndCapacity(bool shouldDecreasePayloadSize = false); + virtual void adjustPayloadStartAndCapacity(qint64 headerSize, bool shouldDecreasePayloadSize = false); qint64 _packetSize = 0; // Total size of the allocated memory std::unique_ptr _packet; // Allocated memory diff --git a/libraries/networking/src/udt/Constants.h b/libraries/networking/src/udt/Constants.h index 5e0f130f13..eaad77d03e 100644 --- a/libraries/networking/src/udt/Constants.h +++ b/libraries/networking/src/udt/Constants.h @@ -25,7 +25,8 @@ namespace udt { static const int UDP_SEND_BUFFER_SIZE_BYTES = 1048576; static const int UDP_RECEIVE_BUFFER_SIZE_BYTES = 1048576; static const int DEFAULT_SYN_INTERVAL_USECS = 10 * 1000; - static const uint32_t CONTROL_BIT_MASK = 1 << (sizeof(SequenceNumber) - 1); + static const int SEQUENCE_NUMBER_BITS = sizeof(SequenceNumber) * 8; + static const uint32_t CONTROL_BIT_MASK = uint32_t(1) << (SEQUENCE_NUMBER_BITS - 1); } #endif // hifi_udt_Constants_h diff --git a/libraries/networking/src/udt/ControlPacket.cpp b/libraries/networking/src/udt/ControlPacket.cpp index 2bc081e197..01d92bfa07 100644 --- a/libraries/networking/src/udt/ControlPacket.cpp +++ b/libraries/networking/src/udt/ControlPacket.cpp @@ -57,7 +57,7 @@ ControlPacket::ControlPacket(Type type) : BasePacket(-1), _type(type) { - adjustPayloadStartAndCapacity(); + adjustPayloadStartAndCapacity(localHeaderSize()); open(QIODevice::ReadWrite); @@ -68,7 +68,7 @@ ControlPacket::ControlPacket(Type type, qint64 size) : BasePacket(localHeaderSize() + size), _type(type) { - adjustPayloadStartAndCapacity(); + adjustPayloadStartAndCapacity(localHeaderSize()); open(QIODevice::ReadWrite); @@ -119,12 +119,14 @@ void ControlPacket::writeType() { ControlBitAndType* bitAndType = reinterpret_cast(_packet.get()); // write the type by OR'ing the new type with the current value & CONTROL_BIT_MASK - *bitAndType = (*bitAndType & CONTROL_BIT_MASK) | (_type << sizeof((ControlPacket::Type) - 1)); + *bitAndType = (*bitAndType & CONTROL_BIT_MASK) | (_type << (sizeof(ControlPacket::Type) * 8 - 1)); } void ControlPacket::readType() { ControlBitAndType bitAndType = *reinterpret_cast(_packet.get()); + Q_ASSERT_X(bitAndType & CONTROL_BIT_MASK, "ControlPacket::readHeader()", "This should be a control packet"); + // read the type uint32_t oversizeType = (uint32_t) (bitAndType & ~CONTROL_BIT_MASK); _type = (Type) oversizeType; diff --git a/libraries/networking/src/udt/Packet.cpp b/libraries/networking/src/udt/Packet.cpp index 96cf5210b4..6ec8ac398e 100644 --- a/libraries/networking/src/udt/Packet.cpp +++ b/libraries/networking/src/udt/Packet.cpp @@ -50,7 +50,7 @@ qint64 Packet::maxPayloadSize() const { } qint64 Packet::totalHeadersSize() const { - return BasePacket::localHeaderSize() + localHeaderSize(); + return BasePacket::totalHeadersSize() + Packet::localHeaderSize(); } qint64 Packet::localHeaderSize() const { @@ -62,7 +62,7 @@ Packet::Packet(qint64 size, bool isReliable, bool isPartOfMessage) : _isReliable(isReliable), _isPartOfMessage(isPartOfMessage) { - adjustPayloadStartAndCapacity(); + adjustPayloadStartAndCapacity(localHeaderSize()); // set the UDT header to default values writeHeader(); @@ -117,14 +117,15 @@ void Packet::writeSequenceNumber(SequenceNumber sequenceNumber) const { writeHeader(); } -static const uint32_t RELIABILITY_BIT_MASK = 1 << (sizeof(Packet::SequenceNumberAndBitField) - 2); -static const uint32_t MESSAGE_BIT_MASK = 1 << (sizeof(Packet::SequenceNumberAndBitField) - 3); +static const uint32_t RELIABILITY_BIT_MASK = uint32_t(1) << (SEQUENCE_NUMBER_BITS - 2); +static const uint32_t MESSAGE_BIT_MASK = uint32_t(1) << (SEQUENCE_NUMBER_BITS - 3); static const uint32_t BIT_FIELD_MASK = CONTROL_BIT_MASK | RELIABILITY_BIT_MASK | MESSAGE_BIT_MASK; void Packet::readHeader() const { SequenceNumberAndBitField seqNumBitField = *reinterpret_cast(_packet.get()); - Q_ASSERT_X((bool) (seqNumBitField & CONTROL_BIT_MASK), - "Packet::readHeader()", "This should be a data packet"); + + Q_ASSERT_X(!(seqNumBitField & CONTROL_BIT_MASK), "Packet::readHeader()", "This should be a data packet"); + _isReliable = (bool) (seqNumBitField & RELIABILITY_BIT_MASK); // Only keep reliability bit _isPartOfMessage = (bool) (seqNumBitField & MESSAGE_BIT_MASK); // Only keep message bit _sequenceNumber = SequenceNumber{ seqNumBitField & ~BIT_FIELD_MASK }; // Remove the bit field diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 5ded97135b..8b18bdddcd 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -16,6 +16,7 @@ #include "../NetworkLogging.h" #include "ControlPacket.h" #include "Packet.h" +#include "../NLPacket.h" using namespace udt; @@ -138,6 +139,8 @@ void Socket::readPendingDatagrams() { // check if this was a control packet or a data packet bool isControlPacket = *buffer & CONTROL_BIT_MASK; + qDebug() << "IS CONTROL" << isControlPacket; + if (isControlPacket) { // setup a control packet from the data we just read auto controlPacket = ControlPacket::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr);