addressed comments

microsoft · Dec 17, 2024 · d2baaa0 · d2baaa0
1 parent 5e48229
commit d2baaa0
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 8 deletions.
diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c
@@ -113,8 +113,12 @@ QuicAckTrackerDidHitReorderingThreshold(
     const uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck);
     uint64_t LargestReported = 0; // The largest packet number that could be declared lost 
 
-    if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold &&
-        Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 <= LargestUnacked) {
+    if (Tracker->LargestPacketNumberAcknowledged != 0 &&
+        Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 > LargestUnacked) {
+        return FALSE;
+    }
+
+    if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold) {
         LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1;
     }
 
@@ -126,18 +130,20 @@ QuicAckTrackerDidHitReorderingThreshold(
     // acknowledgement.
     //
 
-    for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index > 0; --Index) {
-        uint64_t SmallestMissing = 0; // Smallest missing after this range
-        uint64_t HighestMissing = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; // Highest missing in this range
-        if(Index != 0) {
+    for (int Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index >= 0; --Index) {
+        uint64_t SmallestMissing = 0; // Smallest missing in the previous gap
+        uint64_t HighestMissing = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; // Highest missing in the prevous gap
+        if (HighestMissing == 0) {
+            return FALSE;
+        }
+        if (Index != 0) {
             SmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1;
         }
 
         if (LargestReported > SmallestMissing) {
             if (HighestMissing > LargestReported) {
                 SmallestMissing = LargestReported;
-            }
-            else {
+            } else {
                 return FALSE;
             }
         }

diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp
@@ -316,6 +316,13 @@ TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold)
     ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold));
     Tracker.LargestPacketNumberAcknowledged = 9;
 
+    // Case 3
+    ReorderingThreshold = 3;
+    QuicAckTrackerReset(&Tracker); 
+    QuicRangeAddValue(&Tracker.PacketNumbersToAck, 7);
+    QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8);
+    Tracker.LargestPacketNumberAcknowledged = 5;
+    ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold));
 
     // Clean up
     QuicAckTrackerUninitialize(&Tracker);