From 8f2d3182e6c99bdbe3f3af7427b0d610086ea074 Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Wed, 20 Oct 2021 15:50:17 +0200 Subject: [PATCH] [tests] Refactored RCV buffer unit tests (#2170) --- srtcore/buffer.cpp | 2 +- test/test_buffer.cpp | 554 +++++++++++++++++++++++-------------------- 2 files changed, 294 insertions(+), 262 deletions(-) diff --git a/srtcore/buffer.cpp b/srtcore/buffer.cpp index 329b054a0..794066702 100644 --- a/srtcore/buffer.cpp +++ b/srtcore/buffer.cpp @@ -932,7 +932,7 @@ int CRcvBuffer::readBuffer(char* data, int len) { if (m_pUnit[p] == NULL) { - LOGC(brlog.Error, log << CONID() << " IPE readBuffer on null packet pointer"); + LOGC(brlog.Error, log << CONID() << "IPE readBuffer on null packet pointer"); return -1; } diff --git a/test/test_buffer.cpp b/test/test_buffer.cpp index a08aa2406..851fe1167 100644 --- a/test/test_buffer.cpp +++ b/test/test_buffer.cpp @@ -1,267 +1,125 @@ #include +#include #include "gtest/gtest.h" #include "buffer.h" -#include using namespace srt; using namespace std; -// Check the available size of the receiver buffer. -TEST(CRcvBuffer, Create) -{ - const int buffer_size_pkts = 128; - CUnitQueue unit_queue; - CRcvBuffer rcv_buffer(&unit_queue, buffer_size_pkts); - // One buffer cell is always unavailable as it is use to distinguish - // two buffer states: empty and full. - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 1); -} - -// Fill the buffer full, and check adding more data results in an error. -TEST(CRcvBuffer, FullBuffer) +class CRcvBufferReadMsg + : public ::testing::Test { - const int buffer_size_pkts = 16; - CUnitQueue unit_queue; - unit_queue.init(buffer_size_pkts, 1500, AF_INET); - CRcvBuffer rcv_buffer(&unit_queue, buffer_size_pkts); - - const size_t payload_size = 1456; - // Add a number of units (packets) to the buffer - // equal to the buffer size in packets - for (int i = 0; i < rcv_buffer.getAvailBufSize(); ++i) +protected: + CRcvBufferReadMsg() { - CUnit* unit = unit_queue.getNextAvailUnit(); - EXPECT_NE(unit, nullptr); - unit->m_Packet.setLength(payload_size); - EXPECT_EQ(rcv_buffer.addData(unit, i), 0); + // initialization code here } - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 1); // logic - - rcv_buffer.ackData(buffer_size_pkts - 1); - EXPECT_EQ(rcv_buffer.getAvailBufSize(), 0); - - // Try to add more data than the available size of the buffer - CUnit* unit = unit_queue.getNextAvailUnit(); - EXPECT_NE(unit, nullptr); - EXPECT_EQ(rcv_buffer.addData(unit, 1), -1); - - std::array buff; - for (int i = 0; i < buffer_size_pkts - 1; ++i) + ~CRcvBufferReadMsg() { - const int res = rcv_buffer.readBuffer(buff.data(), buff.size()); - EXPECT_EQ(size_t(res), payload_size); + // cleanup any pending stuff, but no exceptions allowed } -} - -// BUG!!! -// In this test case a packet is added to receiver buffer with offset 1, -// thus leaving offset 0 with an empty pointer. -// The buffer says it is not empty, and the data is available -// to be read, but reading should cause error. -TEST(CRcvBuffer, ReadDataIPE) -{ - const int buffer_size_pkts = 16; - CUnitQueue unit_queue; - unit_queue.init(buffer_size_pkts, 1500, AF_INET); - CRcvBuffer rcv_buffer(&unit_queue, buffer_size_pkts); - const size_t payload_size = 1456; - // Add a number of units (packets) to the buffer - // equal to the buffer size in packets - CUnit* unit = unit_queue.getNextAvailUnit(); - EXPECT_NE(unit, nullptr); - unit->m_Packet.setLength(payload_size); - EXPECT_EQ(rcv_buffer.addData(unit, 1), 0); - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 1); - - EXPECT_FALSE(rcv_buffer.isRcvDataAvailable()); - - // BUG. Acknowledging an empty position must not result in a read-readiness. - rcv_buffer.ackData(1); - EXPECT_TRUE(rcv_buffer.isRcvDataAvailable()); - EXPECT_TRUE(rcv_buffer.isRcvDataReady()); - - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 2); - cerr << "Expecting IPE message: \n"; - array buff; - const int res = rcv_buffer.readBuffer(buff.data(), buff.size()); - EXPECT_EQ(res, -1); -} - -TEST(CRcvBuffer, ReadData) -{ - const int buffer_size_pkts = 16; - CUnitQueue unit_queue; - unit_queue.init(buffer_size_pkts, 1500, AF_INET); - CRcvBuffer rcv_buffer(&unit_queue, buffer_size_pkts); - - const size_t payload_size = 1456; - // Add a number of units (packets) to the buffer - // equal to the buffer size in packets - CUnit* unit = unit_queue.getNextAvailUnit(); - EXPECT_NE(unit, nullptr); - unit->m_Packet.setLength(payload_size); - EXPECT_EQ(rcv_buffer.addData(unit, 0), 0); - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 1); - - EXPECT_FALSE(rcv_buffer.isRcvDataAvailable()); - rcv_buffer.ackData(1); - EXPECT_TRUE(rcv_buffer.isRcvDataAvailable()); - - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 2); - - std::array buff; - const int res = rcv_buffer.readBuffer(buff.data(), buff.size()); - EXPECT_EQ(size_t(res), payload_size); -} - -TEST(CRcvBuffer, AddData) -{ - const int buffer_size_pkts = 16; - CUnitQueue unit_queue; - unit_queue.init(buffer_size_pkts, 1500, AF_INET); - CRcvBuffer rcv_buffer(&unit_queue, buffer_size_pkts); - - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 1); // logic - - const size_t payload_size = 1456; - // Add 10 units (packets) to the buffer - for (int i = 0; i < 10; ++i) +protected: + // SetUp() is run immediately before a test starts. + void SetUp() override { - CUnit* unit = unit_queue.getNextAvailUnit(); - EXPECT_NE(unit, nullptr); - unit->m_Packet.setLength(payload_size); - EXPECT_EQ(rcv_buffer.addData(unit, i), 0); + // make_unique is unfortunatelly C++14 + m_unit_queue = unique_ptr(new CUnitQueue); + ASSERT_NE(m_unit_queue.get(), nullptr); + m_unit_queue->init(m_buff_size_pkts, 1500, AF_INET); + m_rcv_buffer = unique_ptr(new CRcvBuffer(m_unit_queue.get(), m_buff_size_pkts)); + ASSERT_NE(m_rcv_buffer.get(), nullptr); } - // The available buffer size remains the same - // The value is reported by SRT receiver like this: - // data[ACKD_BUFFERLEFT] = m_pRcvBuffer->getAvailBufSize(); - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 1); - EXPECT_FALSE(rcv_buffer.isRcvDataAvailable()); - - // Now acknowledge two packets - const int ack_pkts = 2; - rcv_buffer.ackData(2); - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - 1 - ack_pkts); - EXPECT_TRUE(rcv_buffer.isRcvDataAvailable()); - - std::array buff; - for (int i = 0; i < ack_pkts; ++i) + void TearDown() override { - const int res = rcv_buffer.readBuffer(buff.data(), buff.size()); - EXPECT_EQ(size_t(res), payload_size); - EXPECT_EQ(rcv_buffer.getAvailBufSize(), buffer_size_pkts - ack_pkts + i); + // Code here will be called just after the test completes. + // OK to throw exceptions from here if needed. + m_unit_queue.reset(); + m_rcv_buffer.reset(); } - // Add packet to the same position - CUnit* unit = unit_queue.getNextAvailUnit(); - EXPECT_NE(unit, nullptr); - unit->m_Packet.setLength(payload_size); - EXPECT_EQ(rcv_buffer.addData(unit, 1), -1); -} - -TEST(CRcvBuffer, OneMessageInSeveralPackets) -{ - const int buffer_size_pkts = 16; - CUnitQueue unit_queue; - unit_queue.init(buffer_size_pkts, 1500, AF_INET); - CRcvBuffer rcv_buffer(&unit_queue, buffer_size_pkts); - - const int initial_seqno = 1000; - const int message_len_in_pkts = 4; - const size_t payload_size = 1456; - // Add a number of units (packets) to the buffer - // equal to the buffer size in packets - for (int i = 0; i < message_len_in_pkts; ++i) +public: + /// Generate and add one packet to the receiver buffer. + /// + /// @returns the result of rcv_buffer::insert(..) + int addPacket(int seqno, bool pb_first = true, bool pb_last = true, bool out_of_order = false, int ts = 0) { - CUnit* unit = unit_queue.getNextAvailUnit(); + CUnit* unit = m_unit_queue->getNextAvailUnit(); EXPECT_NE(unit, nullptr); CPacket& packet = unit->m_Packet; - packet.setLength(payload_size); - packet.m_iSeqNo = initial_seqno + i; + packet.m_iSeqNo = seqno; + packet.m_iTimeStamp = ts; + + packet.setLength(m_payload_sz); + generatePayload(packet.data(), packet.getLength(), packet.m_iSeqNo); + packet.m_iMsgNo = PacketBoundaryBits(PB_SUBSEQUENT); - if (i == 0) + if (pb_first) packet.m_iMsgNo |= PacketBoundaryBits(PB_FIRST); - const bool is_last_packet = (i == message_len_in_pkts - 1); - if (is_last_packet) + if (pb_last) packet.m_iMsgNo |= PacketBoundaryBits(PB_LAST); - EXPECT_EQ(rcv_buffer.addData(unit, i), 0); + if (!out_of_order) + { + packet.m_iMsgNo |= MSGNO_PACKET_INORDER::wrap(1); + EXPECT_TRUE(packet.getMsgOrderFlag()); + } + + const int offset = CSeqNo::seqoff(m_first_unack_seqno, seqno); + return m_rcv_buffer->addData(unit, offset); } - rcv_buffer.ackData(message_len_in_pkts); + /// @returns 0 on success, the result of rcv_buffer::insert(..) once it failed + int addMessage(size_t msg_len_pkts, int start_seqno, bool out_of_order = false, int ts = 0) + { + for (size_t i = 0; i < msg_len_pkts; ++i) + { + const bool pb_first = (i == 0); + const bool pb_last = (i == (msg_len_pkts - 1)); + const int res = addPacket(start_seqno + i, pb_first, pb_last, out_of_order, ts); - cout << "Buffer size before reading: " << rcv_buffer.getAvailBufSize() << endl; - std::array buff; - cout << "Reading one packet of the 4-packet message" << endl; - const int res = rcv_buffer.readMsg(buff.data(), buff.size()); - EXPECT_EQ(res, payload_size); - cout << "Buffer size after reading: " << rcv_buffer.getAvailBufSize() << endl; -} + if (res != 0) + return res; + } + return 0; + } -class TestRcvBufferRead - : public ::testing::Test -{ -protected: - TestRcvBufferRead() + void generatePayload(char* dst, size_t len, int seqno) { std::iota(dst, dst + len, (char)seqno); } + + bool verifyPayload(char* dst, size_t len, int seqno) { - // initialization code here + // Note. A more consistent way would be to use generatePayload function, + // but I don't want to create another buffer for the data. + for (size_t i = 0; i < len; ++i) + { + if (dst[i] != static_cast(seqno + i)) + return false; + } + return true; } - ~TestRcvBufferRead() + int ackPackets(int num_pkts) { - // cleanup any pending stuff, but no exceptions allowed + m_first_unack_seqno = CSeqNo::incseq(m_first_unack_seqno, num_pkts); + return m_rcv_buffer->ackData(num_pkts); } -protected: - // SetUp() is run immediately before a test starts. - void SetUp() override + int getAvailBufferSize() { - // make_unique is unfortunatelly C++14 - m_unit_queue = unique_ptr(new CUnitQueue); - m_unit_queue->init(m_buff_size_pkts, 1500, AF_INET); - m_rcv_buffer = unique_ptr(new CRcvBuffer(m_unit_queue.get(), m_buff_size_pkts)); + return m_rcv_buffer->getAvailBufSize(); } - void TearDown() override + int readMessage(char* data, size_t len) { - // Code here will be called just after the test completes. - // OK to throw exceptions from here if needed. - m_unit_queue.reset(); - m_rcv_buffer.reset(); + return m_rcv_buffer->readMsg(data, len); } -public: - void addMessage(size_t msg_len_pkts, int start_seqno, bool out_of_order = false) + bool hasAvailablePackets() { - const int msg_offset = start_seqno - m_init_seqno; - - for (size_t i = 0; i < msg_len_pkts; ++i) - { - CUnit* unit = m_unit_queue->getNextAvailUnit(); - EXPECT_NE(unit, nullptr); - - CPacket& packet = unit->m_Packet; - packet.setLength(m_payload_sz); - packet.m_iSeqNo = start_seqno + i; - packet.m_iMsgNo = PacketBoundaryBits(PB_SUBSEQUENT); - if (i == 0) - packet.m_iMsgNo |= PacketBoundaryBits(PB_FIRST); - const bool is_last_packet = (i == (msg_len_pkts - 1)); - if (is_last_packet) - packet.m_iMsgNo |= PacketBoundaryBits(PB_LAST); - - if (!out_of_order) - { - packet.m_iMsgNo |= MSGNO_PACKET_INORDER::wrap(1); - EXPECT_TRUE(packet.getMsgOrderFlag()); - } - - EXPECT_EQ(m_rcv_buffer->addData(unit, msg_offset + i), 0); - } + return m_rcv_buffer->isRcvDataAvailable(); } protected: @@ -269,63 +127,149 @@ class TestRcvBufferRead unique_ptr m_rcv_buffer; const int m_buff_size_pkts = 16; const int m_init_seqno = 1000; + int m_first_unack_seqno = m_init_seqno; static const size_t m_payload_sz = 1456; }; -TEST_F(TestRcvBufferRead, OnePacket) +// Check the available size of the receiver buffer. +TEST_F(CRcvBufferReadMsg, Create) { - const size_t msg_pkts = 1; - // Adding one message without acknowledging - addMessage(msg_pkts, m_init_seqno, false); + EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - 1); +} - const size_t msg_bytelen = msg_pkts * m_payload_sz; - array buff; +// Fill the buffer full, and check adding more data results in an error. +TEST_F(CRcvBufferReadMsg, FullBuffer) +{ + auto& rcv_buffer = *m_rcv_buffer.get(); + // Add a number of units (packets) to the buffer + // equal to the buffer size in packets + for (int i = 0; i < getAvailBufferSize(); ++i) + { + EXPECT_EQ(addMessage(1, CSeqNo::incseq(m_init_seqno, i)), 0); + } + + EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - 1); // logic - EXPECT_FALSE(m_rcv_buffer->isRcvDataAvailable()); + ackPackets(m_buff_size_pkts - 1); + EXPECT_EQ(getAvailBufferSize(), 0); - int res = m_rcv_buffer->readMsg(buff.data(), buff.size()); + // Try to add more data than the available size of the buffer + EXPECT_EQ(addPacket(CSeqNo::incseq(m_init_seqno, getAvailBufferSize())), -1); + + array buff; + for (int i = 0; i < m_buff_size_pkts - 1; ++i) + { + const int res = rcv_buffer.readBuffer(buff.data(), buff.size()); + EXPECT_TRUE(size_t(res) == m_payload_sz); + EXPECT_TRUE(verifyPayload(buff.data(), res, CSeqNo::incseq(m_init_seqno, i))); + } +} + +// BUG in the new RCV buffer!!! +// In this test case a packet is added to receiver buffer with offset 1, +// thus leaving offset 0 with an empty pointer. +// The buffer says it is not empty, and the data is available +// to be read, but reading is not possible. +TEST_F(CRcvBufferReadMsg, OnePacketGap) +{ + // Add one packet message to to the buffer + // with a gap of one packet. + EXPECT_EQ(addMessage(1, CSeqNo::incseq(m_init_seqno)), 0); + + auto& rcv_buffer = *m_rcv_buffer.get(); + // Before ACK the available buffer size stays the same. + EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - 1); + // Not available for reading as not yet acknowledged. + EXPECT_FALSE(hasAvailablePackets()); + // Confirm reading zero bytes. + array buff; + int res = readMessage(buff.data(), buff.size()); EXPECT_EQ(res, 0); - // Full ACK - m_rcv_buffer->ackData(msg_pkts); + // BUG. Acknowledging an empty position must not result in a read-readiness. + ackPackets(1); + // Wrong behavior (BUG) + EXPECT_TRUE(hasAvailablePackets()); + EXPECT_TRUE(rcv_buffer.isRcvDataReady()); - EXPECT_TRUE(m_rcv_buffer->isRcvDataAvailable()); + EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - 2); + cerr << "Expecting IPE from readBuffer(..): \n"; + res = rcv_buffer.readBuffer(buff.data(), buff.size()); + EXPECT_EQ(res, -1); - res = m_rcv_buffer->readMsg(buff.data(), buff.size()); - EXPECT_EQ(res, msg_bytelen); + res = readMessage(buff.data(), buff.size()); + EXPECT_EQ(res, 0); } -TEST_F(TestRcvBufferRead, OnePacketWithGap) +// Add one packet to the buffer and read it once it is acknowledged. +// Confirm the data read is valid. +TEST_F(CRcvBufferReadMsg, OnePacket) { const size_t msg_pkts = 1; // Adding one message without acknowledging - addMessage(msg_pkts, m_init_seqno + 1, false); + addMessage(msg_pkts, m_init_seqno, false); const size_t msg_bytelen = msg_pkts * m_payload_sz; array buff; - EXPECT_FALSE(m_rcv_buffer->isRcvDataAvailable()); + EXPECT_FALSE(hasAvailablePackets()); + const int res1 = readMessage(buff.data(), buff.size()); + EXPECT_EQ(res1, 0); - int res = m_rcv_buffer->readMsg(buff.data(), buff.size()); - EXPECT_EQ(res, 0); + // Full ACK + ackPackets(msg_pkts); + EXPECT_TRUE(hasAvailablePackets()); - // ACK first missing packet - m_rcv_buffer->ackData(msg_pkts); + const int res2 = readMessage(buff.data(), buff.size()); + EXPECT_EQ(res2, msg_bytelen); + EXPECT_TRUE(verifyPayload(buff.data(), res2, m_init_seqno)); +} - EXPECT_TRUE(m_rcv_buffer->isRcvDataAvailable()); +// Add ten packets to the buffer, acknowledge and read some of them. +// Then try to add packets to the position of existing packets. +// We can't check adding to the position of those packets already read, +// because a negative offset is not checked by the receiver buffer, +// but must be handled by the CUDT socket. +TEST_F(CRcvBufferReadMsg, AddData) +{ + const int num_pkts = 10; + ASSERT_LT(num_pkts, m_buff_size_pkts); + for (int i = 0; i < num_pkts; ++i) + { + EXPECT_EQ(addMessage(1, CSeqNo::incseq(m_init_seqno, i)), 0); + } - res = m_rcv_buffer->readMsg(buff.data(), buff.size()); - EXPECT_EQ(res, 0); + // The available buffer size remains the same + // The value is reported by SRT receiver like this: + // data[ACKD_BUFFERLEFT] = m_pRcvBuffer->getAvailBufSize(); + EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - 1); + EXPECT_FALSE(hasAvailablePackets()); - m_rcv_buffer->ackData(msg_pkts); - EXPECT_TRUE(m_rcv_buffer->isRcvDataAvailable()); + // Now acknowledge two packets + const int ack_pkts = 2; + ackPackets(2); + EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - 1 - ack_pkts); + EXPECT_TRUE(hasAvailablePackets()); - res = m_rcv_buffer->readMsg(buff.data(), buff.size()); - EXPECT_EQ(res, msg_bytelen); + std::array buff; + for (int i = 0; i < ack_pkts; ++i) + { + const int res = readMessage(buff.data(), buff.size()); + EXPECT_TRUE(size_t(res) == m_payload_sz); + EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - ack_pkts + i); + EXPECT_TRUE(verifyPayload(buff.data(), res, CSeqNo::incseq(m_init_seqno, i))); + } + + // Add packet to the position of oackets already read. + // Can't check, as negative offset is an error not handled by the receiver buffer. + //EXPECT_EQ(addPacket(m_init_seqno), -1); + + // Add packet to a non-empty position. + EXPECT_EQ(addPacket(CSeqNo::incseq(m_init_seqno, ack_pkts)), -1); } // Check reading the whole message (consisting of several packets) from the buffer. -TEST_F(TestRcvBufferRead, MsgAcked) +TEST_F(CRcvBufferReadMsg, MsgAcked) { const size_t msg_pkts = 4; // Adding one message without acknowledging @@ -335,12 +279,47 @@ TEST_F(TestRcvBufferRead, MsgAcked) array buff; // Acknowledge all packets of the message. - m_rcv_buffer->ackData(4); + ackPackets(msg_pkts); // Now the whole message can be read. EXPECT_TRUE(m_rcv_buffer->isRcvDataReady()); - EXPECT_TRUE(m_rcv_buffer->isRcvDataAvailable()); - const int res = m_rcv_buffer->readMsg(buff.data(), buff.size()); + EXPECT_TRUE(hasAvailablePackets()); + + const int res = readMessage(buff.data(), buff.size()); EXPECT_EQ(res, msg_bytelen); + for (size_t i = 0; i < msg_pkts; ++i) + { + const ptrdiff_t offset = i * m_payload_sz; + EXPECT_TRUE(verifyPayload(buff.data() + offset, m_payload_sz, CSeqNo::incseq(m_init_seqno, i))); + } +} + +// Check reading the whole message (consisting of several packets) into +// a buffer of an insufficient size. +TEST_F(CRcvBufferReadMsg, SmallReadBuffer) +{ + const size_t msg_pkts = 4; + // Adding one message without acknowledging + addMessage(msg_pkts, m_init_seqno, false); + + const size_t msg_bytelen = msg_pkts * m_payload_sz; + array buff; + + // Acknowledge all packets of the message. + ackPackets(msg_pkts); + // Now the whole message can be read. + EXPECT_TRUE(m_rcv_buffer->isRcvDataReady()); + EXPECT_TRUE(hasAvailablePackets()); + + // Check reading into an insufficient size buffer. + // The old buffer extracts the whole message, but copies only + // the number of bytes provided in the 'len' argument. + const int res = readMessage(buff.data(), m_payload_sz); + EXPECT_EQ(res, 1456); + + // No more messages to read + EXPECT_FALSE(m_rcv_buffer->isRcvDataReady()); + EXPECT_FALSE(hasAvailablePackets()); + EXPECT_EQ(getAvailBufferSize(), m_buff_size_pkts - 1); } // BUG!!! @@ -348,7 +327,7 @@ TEST_F(TestRcvBufferRead, MsgAcked) // The RCV buffer implementation has an issue here: when only half of the message is // acknowledged, the RCV buffer signals read-readiness, even though // the message can't be read, and reading returns 0. -TEST_F(TestRcvBufferRead, MsgHalfAck) +TEST_F(CRcvBufferReadMsg, MsgHalfAck) { const size_t msg_pkts = 4; // Adding one message without acknowledging @@ -358,36 +337,89 @@ TEST_F(TestRcvBufferRead, MsgHalfAck) const size_t msg_bytelen = msg_pkts * m_payload_sz; array buff; EXPECT_FALSE(m_rcv_buffer->isRcvDataReady()); - EXPECT_FALSE(m_rcv_buffer->isRcvDataAvailable()); - int res = m_rcv_buffer->readMsg(buff.data(), buff.size()); + EXPECT_FALSE(hasAvailablePackets()); + const int res = readMessage(buff.data(), buff.size()); EXPECT_EQ(res, 0); // ACK half of the message and check read-readiness. - m_rcv_buffer->ackData(2); + ackPackets(2); // FIXME: Sadly RCV buffer says the data is ready to be read. // EXPECT_FALSE(m_rcv_buffer->isRcvDataReady()); - // EXPECT_FALSE(m_rcv_buffer->isRcvDataAvailable()); + // EXPECT_FALSE(hasAvailablePackets()); EXPECT_TRUE(m_rcv_buffer->isRcvDataReady()); - EXPECT_TRUE(m_rcv_buffer->isRcvDataAvailable()); + EXPECT_TRUE(hasAvailablePackets()); // Actually must be nothing to read (can't read half a message). - res = m_rcv_buffer->readMsg(buff.data(), buff.size()); - EXPECT_EQ(res, 0); + const int res2 = readMessage(buff.data(), buff.size()); + EXPECT_EQ(res2, 0); } // BUG!!! // Adding a message with the out-of-order flag set. // RCV buffer does not signal read-readiness, but actually the packet can be read. -TEST_F(TestRcvBufferRead, OutOfOrderMsgNoACK) +TEST_F(CRcvBufferReadMsg, OutOfOrderMsgNoACK) { const size_t msg_pkts = 4; // Adding one message with the Out-Of-Order flag set, but without acknowledging. addMessage(msg_pkts, m_init_seqno, true); EXPECT_FALSE(m_rcv_buffer->isRcvDataReady()); - EXPECT_FALSE(m_rcv_buffer->isRcvDataAvailable()); + EXPECT_FALSE(hasAvailablePackets()); + const size_t msg_bytelen = msg_pkts * m_payload_sz; + array buff; + const int res = readMessage(buff.data(), buff.size()); + EXPECT_EQ(res, msg_bytelen); +} + +// Adding a message with the out-of-order flag set. +// The message can be read. +TEST_F(CRcvBufferReadMsg, OutOfOrderMsgGap) +{ + const size_t msg_pkts = 4; + // Adding one message with the Out-Of-Order flag set, but without acknowledging. + addMessage(msg_pkts, CSeqNo::incseq(m_init_seqno, 1), true); + + EXPECT_FALSE(m_rcv_buffer->isRcvDataReady()); + EXPECT_FALSE(hasAvailablePackets()); const size_t msg_bytelen = msg_pkts * m_payload_sz; array buff; - const int res = m_rcv_buffer->readMsg(buff.data(), buff.size()); + const int res = readMessage(buff.data(), buff.size()); EXPECT_EQ(res, msg_bytelen); + for (size_t i = 0; i < msg_pkts; ++i) + { + const ptrdiff_t offset = i * m_payload_sz; + EXPECT_TRUE(verifyPayload(buff.data() + offset, m_payload_sz, CSeqNo::incseq(m_init_seqno, 1 + i))); + } + + EXPECT_FALSE(m_rcv_buffer->isRcvDataReady()); + EXPECT_FALSE(hasAvailablePackets()); + // Adding one message with the Out-Of-Order flag set, but without acknowledging. + //int seqno, bool pb_first = true, bool pb_last = true, bool out_of_order = false, int ts = 0) + const int res2 = addPacket(CSeqNo::incseq(m_init_seqno, 1)); + EXPECT_EQ(res2, -1); // already exists + + EXPECT_EQ(addPacket(m_init_seqno), 0); + ackPackets(msg_pkts + 1); + EXPECT_TRUE(m_rcv_buffer->isRcvDataReady()); + EXPECT_TRUE(hasAvailablePackets()); + + const int res3 = readMessage(buff.data(), buff.size()); + EXPECT_TRUE(res3 == m_payload_sz); + EXPECT_TRUE(verifyPayload(buff.data(), m_payload_sz, m_init_seqno)); + + // Only "passack" packets remain in the buffer. + // They are falsely signalled as read-ready. + EXPECT_TRUE(m_rcv_buffer->isRcvDataReady()); // BUG: nothing to read. + EXPECT_TRUE(hasAvailablePackets()); // BUG: nothing to read. + + // Adding a packet right after the EntryState_Read packets. + const int seqno = CSeqNo::incseq(m_init_seqno, msg_pkts + 1); + EXPECT_EQ(addPacket(seqno), 0); + ackPackets(1); + EXPECT_TRUE(m_rcv_buffer->isRcvDataReady()); + EXPECT_TRUE(hasAvailablePackets()); + EXPECT_TRUE(readMessage(buff.data(), buff.size()) == m_payload_sz); + EXPECT_TRUE(verifyPayload(buff.data(), m_payload_sz, seqno)); + EXPECT_FALSE(m_rcv_buffer->isRcvDataReady()); + EXPECT_FALSE(hasAvailablePackets()); }