Skip to content

Commit

Permalink
Fixed wrong usage of seqcmp
Browse files Browse the repository at this point in the history
  • Loading branch information
@rndi
Mikołaj Małecki authored and rndi committed Feb 27, 2019
1 parent 6398508 commit 2884047
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 21 deletions.
8 changes: 4 additions & 4 deletions srtcore/common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ class StatsLossRecords

void add(int32_t lo, int32_t hi)
{
int32_t end = lo + CSeqNo::seqcmp(hi, lo);
int32_t end = CSeqNo::incseq(hi);
for (int32_t i = lo; i != end; i = CSeqNo::incseq(i))
add(i);
}
Expand All @@ -701,7 +701,7 @@ class StatsLossRecords
}

// Calculate the distance between this seq and the oldest one.
int seqdiff = CSeqNo::seqcmp(seq, initseq);
int seqdiff = CSeqNo::seqoff(initseq, seq);
if ( seqdiff > int(SIZE) )
{
// Size exceeded. Drop the oldest sequences.
Expand Down Expand Up @@ -739,7 +739,7 @@ class StatsLossRecords
void remove(int32_t seq)
{
// Check if is in range. If not, ignore.
int seqdiff = CSeqNo::seqcmp(seq, initseq);
int seqdiff = CSeqNo::seqoff(initseq, seq);
if ( seqdiff < 0 )
return; // already out of array
if ( seqdiff > SIZE )
Expand All @@ -750,7 +750,7 @@ class StatsLossRecords

bool find(int32_t seq) const
{
int seqdiff = CSeqNo::seqcmp(seq, initseq);
int seqdiff = CSeqNo::seqoff(initseq, seq);
if ( seqdiff < 0 )
return false; // already out of array
if ( size_t(seqdiff) > SIZE )
Expand Down
28 changes: 15 additions & 13 deletions srtcore/core.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6257,10 +6257,10 @@ static void DebugAck(string hdr, int prev, int ack)
}

prev = CSeqNo::incseq(prev);
int diff = CSeqNo::seqcmp(ack, prev);
int diff = CSeqNo::seqoff(prev, ack);
if ( diff < 0 )
{
HLOGC(mglog.Debug, log << hdr << "ACK ERROR: " << prev << "-" << ack << "(diff " << CSeqNo::seqcmp(ack, prev) << ")");
HLOGC(mglog.Debug, log << hdr << "ACK ERROR: " << prev << "-" << ack << "(diff " << diff << ")");
return;
}

Expand Down Expand Up @@ -6648,12 +6648,12 @@ void CUDT::processCtrl(CPacket& ctrlpkt)
CGuard::enterCS(m_AckLock);

// check the validation of the ack
int seqdiff = CSeqNo::seqcmp(ack, CSeqNo::incseq(m_iSndCurrSeqNo));
if (seqdiff> 0)
if (CSeqNo::seqcmp(ack, CSeqNo::incseq(m_iSndCurrSeqNo)) > 0)
{
CGuard::leaveCS(m_AckLock);
//this should not happen: attack or bug
LOGC(glog.Error, log << CONID() << "ATTACK/IPE: incoming ack seq " << ack << " exceeds current " << m_iSndCurrSeqNo << " by " << seqdiff << "!");
LOGC(glog.Error, log << CONID() << "ATTACK/IPE: incoming ack seq " << ack << " exceeds current "
<< m_iSndCurrSeqNo << " by " << (CSeqNo::seqoff(m_iSndCurrSeqNo, ack)-1) << "!");
m_bBroken = true;
m_iBrokenCounter = 0;
break;
Expand Down Expand Up @@ -6909,7 +6909,8 @@ void CUDT::processCtrl(CPacket& ctrlpkt)
// <lo, hi> specification means that the consecutive cell has been already interpreted.
++ i;

HLOGF(mglog.Debug, "received UMSG_LOSSREPORT: %d-%d (%d packets)...", losslist_lo, losslist_hi, CSeqNo::seqcmp(losslist_hi, losslist_lo)+1);
HLOGF(mglog.Debug, "received UMSG_LOSSREPORT: %d-%d (%d packets)...",
losslist_lo, losslist_hi, CSeqNo::seqoff(losslist_lo, losslist_hi)+1);

if ((CSeqNo::seqcmp(losslist_lo, losslist_hi) > 0) || (CSeqNo::seqcmp(losslist_hi, m_iSndCurrSeqNo) > 0))
{
Expand Down Expand Up @@ -7756,7 +7757,8 @@ int CUDT::processData(CUnit* unit)
// pack loss list for (possibly belated) NAK
// The LOSSREPORT will be sent in a while.
m_FreshLoss.push_back(CRcvFreshLoss(seqlo, seqhi, initial_loss_ttl));
HLOGF(mglog.Debug, "added loss sequence %d-%d (%d) with tolerance %d", seqlo, seqhi, 1+CSeqNo::seqcmp(seqhi, seqlo), initial_loss_ttl);
HLOGF(mglog.Debug, "added loss sequence %d-%d (%d) with tolerance %d", seqlo, seqhi,
1+CSeqNo::seqoff(seqlo, seqhi), initial_loss_ttl);
}
else
{
Expand All @@ -7770,7 +7772,7 @@ int CUDT::processData(CUnit* unit)
seq[0] |= LOSSDATA_SEQNO_RANGE_FIRST;
sendCtrl(UMSG_LOSSREPORT, NULL, seq, 2);
}
HLOGF(mglog.Debug, "lost packets %d-%d (%d packets): sending LOSSREPORT", seqlo, seqhi, 1+CSeqNo::seqcmp(seqhi, seqlo));
HLOGF(mglog.Debug, "lost packets %d-%d (%d packets): sending LOSSREPORT", seqlo, seqhi, 1+CSeqNo::seqoff(seqlo, seqhi));
}

int loss = CSeqNo::seqlen(m_iRcvCurrSeqNo, packet.m_iSeqNo) - 2;
Expand Down Expand Up @@ -7824,7 +7826,7 @@ int CUDT::processData(CUnit* unit)
for( ; i != m_FreshLoss.end() && i->ttl <= 0; ++i )
{
HLOGF(mglog.Debug, "Packet seq %d-%d (%d packets) considered lost - sending LOSSREPORT",
i->seq[0], i->seq[1], CSeqNo::seqcmp(i->seq[1], i->seq[0])+1);
i->seq[0], i->seq[1], CSeqNo::seqoff(i->seq[0], i->seq[1])+1);
addLossRecord(lossdata, i->seq[0], i->seq[1]);
}

Expand All @@ -7842,7 +7844,7 @@ int CUDT::processData(CUnit* unit)
else
{
HLOGF(mglog.Debug, "STILL %" PRIzu " FRESH LOSS RECORDS, FIRST: %d-%d (%d) TTL: %d", m_FreshLoss.size(),
i->seq[0], i->seq[1], 1+CSeqNo::seqcmp(i->seq[1], i->seq[0]),
i->seq[0], i->seq[1], 1+CSeqNo::seqoff(i->seq[0], i->seq[1]),
i->ttl);
}

Expand Down Expand Up @@ -7936,7 +7938,7 @@ void CUDT::unlose(const CPacket& packet)
{
HLOGF(mglog.Debug, "received out-of-band packet seq %d", sequence);

int seqdiff = abs(CSeqNo::seqcmp(m_iRcvCurrSeqNo, packet.m_iSeqNo));
int seqdiff = abs(CSeqNo::seqoff(packet.m_iSeqNo, m_iRcvCurrSeqNo));
m_iTraceReorderDistance = max(seqdiff, m_iTraceReorderDistance);
if ( seqdiff > m_iReorderTolerance )
{
Expand Down Expand Up @@ -8553,7 +8555,7 @@ void CUDT::checkTimers()
m_iSndLossTotal += 1; // num;

HLOGC(mglog.Debug, log << CONID() << "ENFORCED LATEREXMIT by ACK-TMOUT (scheduling): " << CSeqNo::incseq(m_iSndLastAck) << "-" << csn
<< " (" << CSeqNo::seqcmp(csn, m_iSndLastAck) << " packets)");
<< " (" << CSeqNo::seqoff(m_iSndLastAck, csn) << " packets)");
}
}
// protect packet retransmission
Expand Down Expand Up @@ -8612,7 +8614,7 @@ void CUDT::checkTimers()
int32_t csn = m_iSndCurrSeqNo;
int num = m_pSndLossList->insert(m_iSndLastAck, csn);
HLOGC(mglog.Debug, log << CONID() << "ENFORCED FASTREXMIT by ACK-TMOUT PREPARED: " << m_iSndLastAck << "-" << csn
<< " (" << CSeqNo::seqcmp(csn, m_iSndLastAck) << " packets)");
<< " (" << CSeqNo::seqoff(m_iSndLastAck, csn) << " packets)");

HLOGC(mglog.Debug, log << "timeout lost: pkts=" << num << " rtt+4*var=" <<
m_iRTT + 4 * m_iRTTVar << " cnt=" << m_iReXmitCount << " diff="
Expand Down
7 changes: 3 additions & 4 deletions srtcore/smoother.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,9 +491,8 @@ class FileSmoother: public SmootherBase
// using FileSmoother, so relying on SRTO_TRANSTYPE rather than
// just SRTO_SMOOTHER is recommended.
int32_t lossbegin = SEQNO_VALUE::unwrap(losslist[0]);
int seqdiff = CSeqNo::seqcmp(lossbegin, m_iLastDecSeq);

if (seqdiff > 0)
if (CSeqNo::seqcmp(lossbegin, m_iLastDecSeq) > 0)
{
m_dLastDecPeriod = m_dPktSndPeriod;
m_dPktSndPeriod = ceil(m_dPktSndPeriod * 1.125);
Expand Down Expand Up @@ -521,7 +520,7 @@ class FileSmoother: public SmootherBase
m_iLastDecSeq = m_parent->sndSeqNo();
HLOGC(mglog.Debug, log << "FileSmoother: LOSS:PERIOD lseq=" << lossbegin
<< ", dseq=" << m_iLastDecSeq
<< ", seqdiff=" << seqdiff
<< ", seqdiff=" << CSeqNo::seqoff(m_iLastDecSeq, lossbegin)
<< ", deccnt=" << m_iDecCount
<< ", decrnd=" << m_iDecRandom
<< ", sndperiod=" << m_dPktSndPeriod << "us");
Expand All @@ -530,7 +529,7 @@ class FileSmoother: public SmootherBase
{
HLOGC(mglog.Debug, log << "FileSmoother: LOSS:STILL lseq=" << lossbegin
<< ", dseq=" << m_iLastDecSeq
<< ", seqdiff=" << seqdiff
<< ", seqdiff=" << CSeqNo::seqoff(m_iLastDecSeq, lossbegin)
<< ", deccnt=" << m_iDecCount
<< ", decrnd=" << m_iDecRandom
<< ", sndperiod=" << m_dPktSndPeriod << "us");
Expand Down

0 comments on commit 2884047

Please sign in to comment.