From fc763e1f07d209115980b23a6ff189eee50e6d32 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Fri, 22 Sep 2017 22:36:46 +0900
Subject: [PATCH] Free ngtcp2_acktr_entry in ngtcp2_acktr_free

---
 lib/ngtcp2_acktr.c        | 14 ++++++++++++++
 lib/ngtcp2_acktr.h        |  8 ++++----
 lib/ngtcp2_conn.c         | 12 ------------
 tests/ngtcp2_acktr_test.c | 31 ++++++++++++++-----------------
 4 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/lib/ngtcp2_acktr.c b/lib/ngtcp2_acktr.c
index 12331e22..1e9e47e9 100644
--- a/lib/ngtcp2_acktr.c
+++ b/lib/ngtcp2_acktr.c
@@ -66,11 +66,19 @@ int ngtcp2_acktr_init(ngtcp2_acktr *acktr, ngtcp2_mem *mem) {
 }
 
 void ngtcp2_acktr_free(ngtcp2_acktr *acktr) {
+  ngtcp2_acktr_entry *ent, *next;
+
   if (acktr == NULL) {
     return;
   }
 
   ngtcp2_ringbuf_free(&acktr->acks);
+
+  for (ent = acktr->ent; ent;) {
+    next = ent->next;
+    ngtcp2_acktr_entry_del(ent, acktr->mem);
+    ent = next;
+  }
 }
 
 int ngtcp2_acktr_add(ngtcp2_acktr *acktr, ngtcp2_acktr_entry *ent,
@@ -142,6 +150,10 @@ ngtcp2_acktr_entry **ngtcp2_acktr_get(ngtcp2_acktr *acktr) {
 }
 
 void ngtcp2_acktr_pop(ngtcp2_acktr *acktr) {
+  ngtcp2_acktr_entry *ent = acktr->ent;
+
+  assert(acktr->ent);
+
   --acktr->nack;
   acktr->ent = acktr->ent->next;
   if (acktr->ent) {
@@ -149,6 +161,8 @@ void ngtcp2_acktr_pop(ngtcp2_acktr *acktr) {
   } else {
     acktr->tail = NULL;
   }
+
+  ngtcp2_acktr_entry_del(ent, acktr->mem);
 }
 
 void ngtcp2_acktr_add_ack(ngtcp2_acktr *acktr, uint64_t pkt_num,
diff --git a/lib/ngtcp2_acktr.h b/lib/ngtcp2_acktr.h
index 2f78af19..d45662bd 100644
--- a/lib/ngtcp2_acktr.h
+++ b/lib/ngtcp2_acktr.h
@@ -95,8 +95,8 @@ typedef struct {
 int ngtcp2_acktr_init(ngtcp2_acktr *acktr, ngtcp2_mem *mem);
 
 /*
- * ngtcp2_acktr_free frees resources allocated for |acktr|.  It does
- * not free any ngtcp2_acktr_entry directly or indirectly pointed by
+ * ngtcp2_acktr_free frees resources allocated for |acktr|.  It frees
+ * any ngtcp2_acktr_entry directly or indirectly pointed by
  * acktr->ent.
  */
 void ngtcp2_acktr_free(ngtcp2_acktr *acktr);
@@ -128,8 +128,8 @@ void ngtcp2_acktr_forget(ngtcp2_acktr *acktr, ngtcp2_acktr_entry *ent);
 ngtcp2_acktr_entry **ngtcp2_acktr_get(ngtcp2_acktr *acktr);
 
 /*
- * ngtcp2_acktr_remove the head of entries, which has the largest
- * packet number.
+ * ngtcp2_acktr_removes and frees the head of entries, which has the
+ * largest packet number.
  */
 void ngtcp2_acktr_pop(ngtcp2_acktr *acktr);
 
diff --git a/lib/ngtcp2_conn.c b/lib/ngtcp2_conn.c
index ef2d7268..314cbeb8 100644
--- a/lib/ngtcp2_conn.c
+++ b/lib/ngtcp2_conn.c
@@ -273,16 +273,6 @@ int ngtcp2_conn_server_new(ngtcp2_conn **pconn, uint64_t conn_id,
   return conn_new(pconn, conn_id, version, callbacks, settings, user_data, 1);
 }
 
-static void delete_acktr_entry(ngtcp2_acktr_entry *ent, ngtcp2_mem *mem) {
-  ngtcp2_acktr_entry *next;
-
-  for (; ent;) {
-    next = ent->next;
-    ngtcp2_acktr_entry_del(ent, mem);
-    ent = next;
-  }
-}
-
 static void delete_buffed_pkts(ngtcp2_pkt_chain *pc, ngtcp2_mem *mem) {
   ngtcp2_pkt_chain *next;
 
@@ -321,7 +311,6 @@ void ngtcp2_conn_del(ngtcp2_conn *conn) {
 
   delete_buffed_pkts(conn->buffed_rx_ppkts, conn->mem);
 
-  delete_acktr_entry(conn->acktr.ent, conn->mem);
   ngtcp2_acktr_free(&conn->acktr);
 
   ngtcp2_crypto_km_del(conn->rx_ckm, conn->mem);
@@ -1861,7 +1850,6 @@ static int conn_recv_server_stateless_retry(ngtcp2_conn *conn) {
   conn->max_rx_pkt_num = 0;
 
   ngtcp2_rtb_free(&conn->rtb);
-  delete_acktr_entry(conn->acktr.ent, conn->mem);
   ngtcp2_acktr_free(&conn->acktr);
   ngtcp2_map_remove(&conn->strms, 0);
   ngtcp2_strm_free(conn->strm0);
diff --git a/tests/ngtcp2_acktr_test.c b/tests/ngtcp2_acktr_test.c
index 7d82482f..e8bd8799 100644
--- a/tests/ngtcp2_acktr_test.c
+++ b/tests/ngtcp2_acktr_test.c
@@ -31,11 +31,7 @@
 
 void test_ngtcp2_acktr_add(void) {
   ngtcp2_acktr acktr;
-  ngtcp2_acktr_entry ents[] = {
-      {NULL, NULL, 1, 1000}, {NULL, NULL, 5, 1001}, {NULL, NULL, 7, 1002},
-      {NULL, NULL, 4, 1003}, {NULL, NULL, 6, 1004}, {NULL, NULL, 2, 1005},
-      {NULL, NULL, 3, 1006},
-  };
+  ngtcp2_acktr_entry *ents[7];
   uint64_t max_pkt_num[] = {1, 5, 7, 7, 7, 7, 7};
   ngtcp2_acktr_entry **pent;
   size_t i;
@@ -44,8 +40,16 @@ void test_ngtcp2_acktr_add(void) {
 
   ngtcp2_acktr_init(&acktr, mem);
 
+  ngtcp2_acktr_entry_new(&ents[0], 1, 1000, mem);
+  ngtcp2_acktr_entry_new(&ents[1], 5, 1001, mem);
+  ngtcp2_acktr_entry_new(&ents[2], 7, 1002, mem);
+  ngtcp2_acktr_entry_new(&ents[3], 4, 1003, mem);
+  ngtcp2_acktr_entry_new(&ents[4], 6, 1004, mem);
+  ngtcp2_acktr_entry_new(&ents[5], 2, 1005, mem);
+  ngtcp2_acktr_entry_new(&ents[6], 3, 1006, mem);
+
   for (i = 0; i < arraylen(ents); ++i) {
-    rv = ngtcp2_acktr_add(&acktr, &ents[i], 1);
+    rv = ngtcp2_acktr_add(&acktr, ents[i], 1);
 
     CU_ASSERT(0 == rv);
 
@@ -70,12 +74,13 @@ void test_ngtcp2_acktr_add(void) {
 
   /* Check duplicates */
   ngtcp2_acktr_init(&acktr, mem);
+  ngtcp2_acktr_entry_new(&ents[0], 1, 1000, mem);
 
-  rv = ngtcp2_acktr_add(&acktr, &ents[0], 1);
+  rv = ngtcp2_acktr_add(&acktr, ents[0], 1);
 
   CU_ASSERT(0 == rv);
 
-  rv = ngtcp2_acktr_add(&acktr, &ents[0], 1);
+  rv = ngtcp2_acktr_add(&acktr, ents[0], 1);
 
   CU_ASSERT(NGTCP2_ERR_PROTO == rv);
 
@@ -108,7 +113,6 @@ void test_ngtcp2_acktr_eviction(void) {
 
     CU_ASSERT(NGTCP2_ACKTR_MAX_ENT + extra - i - 1 == ent->pkt_num);
 
-    ngtcp2_acktr_entry_del(ent, mem);
     ent = next;
   }
 
@@ -134,7 +138,6 @@ void test_ngtcp2_acktr_eviction(void) {
 
     CU_ASSERT(NGTCP2_ACKTR_MAX_ENT + extra - i - 1 == ent->pkt_num);
 
-    ngtcp2_acktr_entry_del(ent, mem);
     ent = next;
   }
 
@@ -179,7 +182,7 @@ void test_ngtcp2_acktr_recv_ack(void) {
   uint64_t pkt_nums[] = {
       4500, 4499, 4497, 4496, 4494, 4493, 4491, 4490, 4488, 4487,
   };
-  ngtcp2_acktr_entry *ent, *next;
+  ngtcp2_acktr_entry *ent;
 
   ngtcp2_acktr_init(&acktr, mem);
 
@@ -226,10 +229,4 @@ void test_ngtcp2_acktr_recv_ack(void) {
   CU_ASSERT(ent == acktr.tail);
 
   ngtcp2_acktr_free(&acktr);
-
-  for (ent = acktr.ent; ent;) {
-    next = ent->next;
-    ngtcp2_acktr_entry_del(ent, mem);
-    ent = next;
-  }
 }
-- 
GitLab