Drop m_list_size from ReliablePacketBuffer

It's not required and, worse, can lead to bugs.
This commit is contained in:
sfan5 2019-08-15 19:54:40 +02:00
parent d7c10b66d3
commit fc2f55d931
2 changed files with 21 additions and 20 deletions

@ -169,6 +169,7 @@ void ReliablePacketBuffer::print()
index++; index++;
} }
} }
bool ReliablePacketBuffer::empty() bool ReliablePacketBuffer::empty()
{ {
MutexAutoLock listlock(m_list_mutex); MutexAutoLock listlock(m_list_mutex);
@ -177,7 +178,8 @@ bool ReliablePacketBuffer::empty()
u32 ReliablePacketBuffer::size() u32 ReliablePacketBuffer::size()
{ {
return m_list_size; MutexAutoLock listlock(m_list_mutex);
return m_list.size();
} }
bool ReliablePacketBuffer::containsPacket(u16 seqnum) bool ReliablePacketBuffer::containsPacket(u16 seqnum)
@ -198,17 +200,19 @@ RPBSearchResult ReliablePacketBuffer::findPacket(u16 seqnum)
} }
return i; return i;
} }
RPBSearchResult ReliablePacketBuffer::notFound() RPBSearchResult ReliablePacketBuffer::notFound()
{ {
return m_list.end(); return m_list.end();
} }
bool ReliablePacketBuffer::getFirstSeqnum(u16& result) bool ReliablePacketBuffer::getFirstSeqnum(u16& result)
{ {
MutexAutoLock listlock(m_list_mutex); MutexAutoLock listlock(m_list_mutex);
if (m_list.empty()) if (m_list.empty())
return false; return false;
BufferedPacket p = *m_list.begin(); const BufferedPacket &p = *m_list.begin();
result = readU16(&p.data[BASE_HEADER_SIZE+1]); result = readU16(&p.data[BASE_HEADER_SIZE + 1]);
return true; return true;
} }
@ -219,16 +223,16 @@ BufferedPacket ReliablePacketBuffer::popFirst()
throw NotFoundException("Buffer is empty"); throw NotFoundException("Buffer is empty");
BufferedPacket p = *m_list.begin(); BufferedPacket p = *m_list.begin();
m_list.erase(m_list.begin()); m_list.erase(m_list.begin());
--m_list_size;
if (m_list_size == 0) { if (m_list.empty()) {
m_oldest_non_answered_ack = 0; m_oldest_non_answered_ack = 0;
} else { } else {
m_oldest_non_answered_ack = m_oldest_non_answered_ack =
readU16(&(*m_list.begin()).data[BASE_HEADER_SIZE+1]); readU16(&m_list.begin()->data[BASE_HEADER_SIZE + 1]);
} }
return p; return p;
} }
BufferedPacket ReliablePacketBuffer::popSeqnum(u16 seqnum) BufferedPacket ReliablePacketBuffer::popSeqnum(u16 seqnum)
{ {
MutexAutoLock listlock(m_list_mutex); MutexAutoLock listlock(m_list_mutex);
@ -249,15 +253,17 @@ BufferedPacket ReliablePacketBuffer::popSeqnum(u16 seqnum)
} }
m_list.erase(r); m_list.erase(r);
--m_list_size;
if (m_list_size == 0) if (m_list.empty()) {
{ m_oldest_non_answered_ack = 0; } m_oldest_non_answered_ack = 0;
else } else {
{ m_oldest_non_answered_ack = readU16(&(*m_list.begin()).data[BASE_HEADER_SIZE+1]); } m_oldest_non_answered_ack =
readU16(&m_list.begin()->data[BASE_HEADER_SIZE + 1]);
}
return p; return p;
} }
void ReliablePacketBuffer::insert(BufferedPacket &p,u16 next_expected)
void ReliablePacketBuffer::insert(BufferedPacket &p, u16 next_expected)
{ {
MutexAutoLock listlock(m_list_mutex); MutexAutoLock listlock(m_list_mutex);
if (p.data.getSize() < BASE_HEADER_SIZE + 3) { if (p.data.getSize() < BASE_HEADER_SIZE + 3) {
@ -284,8 +290,7 @@ void ReliablePacketBuffer::insert(BufferedPacket &p,u16 next_expected)
return; return;
} }
++m_list_size; sanity_check(m_list.size() <= SEQNUM_MAX); // FIXME: Handle the error?
sanity_check(m_list_size <= SEQNUM_MAX+1); // FIXME: Handle the error?
// Find the right place for the packet and insert it there // Find the right place for the packet and insert it there
// If list is empty, just add it // If list is empty, just add it
@ -324,8 +329,6 @@ void ReliablePacketBuffer::insert(BufferedPacket &p,u16 next_expected)
if (s == seqnum) { if (s == seqnum) {
/* nothing to do this seems to be a resent packet */ /* nothing to do this seems to be a resent packet */
/* for paranoia reason data should be compared */ /* for paranoia reason data should be compared */
--m_list_size;
if ( if (
(readU16(&(i->data[BASE_HEADER_SIZE+1])) != seqnum) || (readU16(&(i->data[BASE_HEADER_SIZE+1])) != seqnum) ||
(i->data.getSize() != p.data.getSize()) || (i->data.getSize() != p.data.getSize()) ||
@ -348,8 +351,7 @@ void ReliablePacketBuffer::insert(BufferedPacket &p,u16 next_expected)
/* insert or push back */ /* insert or push back */
else if (i != m_list.end()) { else if (i != m_list.end()) {
m_list.insert(i, p); m_list.insert(i, p);
} } else {
else {
m_list.push_back(p); m_list.push_back(p);
} }

@ -240,7 +240,7 @@ public:
BufferedPacket popFirst(); BufferedPacket popFirst();
BufferedPacket popSeqnum(u16 seqnum); BufferedPacket popSeqnum(u16 seqnum);
void insert(BufferedPacket &p,u16 next_expected); void insert(BufferedPacket &p, u16 next_expected);
void incrementTimeouts(float dtime); void incrementTimeouts(float dtime);
std::list<BufferedPacket> getTimedOuts(float timeout, std::list<BufferedPacket> getTimedOuts(float timeout,
@ -257,7 +257,6 @@ private:
RPBSearchResult findPacket(u16 seqnum); RPBSearchResult findPacket(u16 seqnum);
std::list<BufferedPacket> m_list; std::list<BufferedPacket> m_list;
u32 m_list_size = 0;
u16 m_oldest_non_answered_ack; u16 m_oldest_non_answered_ack;