From 0e447dc3bee01abb3b9402786066ecb55fa43c3b Mon Sep 17 00:00:00 2001
From: Stephen Seo <seo.disparate@gmail.com>
Date: Tue, 4 Jun 2024 15:08:33 +0900
Subject: [PATCH] Refactorings

Internally use doubly-linked list instead of singly-linked list. Also
minor fixes related to changing use to doubly-linked list.

Removed unnecessary comments.

Move list add/remove code to designated functions.
---
 src/another_memcheck.cc          | 72 ++++++++++++++++++++------------
 src/another_memcheck.h           | 11 ++++-
 src/another_memcheck_external.cc |  2 -
 3 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/src/another_memcheck.cc b/src/another_memcheck.cc
index 1ddcffd..cdc93b6 100644
--- a/src/another_memcheck.cc
+++ b/src/another_memcheck.cc
@@ -23,13 +23,45 @@ namespace SC_AM_Internal {
   Malloced::Malloced() : address(nullptr), size(0) {}
   Malloced::Malloced(void *address, std::size_t size) : address(address), size(size) {}
 
-  Stats::Stats() : malloced_list(nullptr) {
+  void ListNode::add_to_list(ListNode *tail, Malloced *data) {
+    ListNode *new_node = reinterpret_cast<ListNode*>(real_malloc(sizeof(ListNode)));
+    new_node->next = tail;
+    new_node->prev = tail->prev;
+    tail->prev->next = new_node;
+    tail->prev = new_node;
+
+    new_node->data = data;
+  }
+
+  bool ListNode::remove_from_list(ListNode *head, void *ptr) {
+    ListNode *node = head;
+    while (node != nullptr) {
+      node = node->next;
+      if (node->data && node->data->address == ptr) {
+        node->prev->next = node->next;
+        node->next->prev = node->prev;
+        real_free(node->data);
+        real_free(node);
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  Stats::Stats() : malloced_list_head(nullptr), malloced_list_tail(nullptr) {
   }
 
   void Stats::initialize() {
-    malloced_list = reinterpret_cast<ListNode*>(real_malloc(sizeof(ListNode)));
-    malloced_list->next = nullptr;
-    malloced_list->data = nullptr;
+    malloced_list_head = reinterpret_cast<ListNode*>(real_malloc(sizeof(ListNode)));
+    malloced_list_tail = reinterpret_cast<ListNode*>(real_malloc(sizeof(ListNode)));
+    malloced_list_head->next = malloced_list_tail;
+    malloced_list_head->prev = nullptr;
+    malloced_list_tail->next = nullptr;
+    malloced_list_tail->prev = malloced_list_head;
+    malloced_list_head->data = nullptr;
+    malloced_list_tail->data = nullptr;
+
     on_exit([] ([[maybe_unused]] int status, void *ptr) {
       const Stats *stats = reinterpret_cast<const Stats*>(ptr);
       stats->print_status();
@@ -39,15 +71,10 @@ namespace SC_AM_Internal {
   void *Stats::do_malloc(std::size_t size) {
     void *address = real_malloc(size);
     if (address != nullptr) {
-      auto *node = malloced_list;
-      while (node->next != nullptr) {
-        node = node->next;
-      }
-      node->next = reinterpret_cast<ListNode*>(real_malloc(sizeof(ListNode)));
-      node->next->next = nullptr;
-      node->next->data = reinterpret_cast<Malloced*>(real_malloc(sizeof(Malloced)));
-      node->next->data->address = address;
-      node->next->data->size = size;
+      Malloced *data = reinterpret_cast<Malloced*>(real_malloc(sizeof(Malloced)));
+      data->address = address;
+      data->size = size;
+      ListNode::add_to_list(malloced_list_tail, data);
     }
 
     return address;
@@ -55,17 +82,8 @@ namespace SC_AM_Internal {
 
   void Stats::do_free(void *ptr) {
     if (ptr) {
-      auto *node = malloced_list;
-      decltype(node) parent_node;
-      while (node->next != nullptr) {
-        parent_node = node;
-        node = node->next;
-        if (node->data && node->data->address == ptr) {
-          real_free(node->data);
-          parent_node->next = node->next;
-          real_free(node);
-          break;
-        }
+      if(!ListNode::remove_from_list(malloced_list_head, ptr)) {
+        std::clog << "WARNING: Attempted free of unknown memory location!\n";
       }
     }
   }
@@ -75,11 +93,13 @@ namespace SC_AM_Internal {
     // it is expected for the OS to reclaim memory from stopped processes and
     // this function only runs at the end of program execution.
     std::clog << "List of unfreed memory:\n";
-    auto *node = malloced_list;
+    auto *node = malloced_list_head;
     while (node->next != nullptr) {
       node = node->next;
 
-      std::clog << "  " << node->data->address << ": size " << node->data->size << '\n';
+      if (node->data) {
+        std::clog << "  " << node->data->address << ": size " << node->data->size << '\n';
+      }
     }
   }
 }
diff --git a/src/another_memcheck.h b/src/another_memcheck.h
index 69b0bf7..6e96a33 100644
--- a/src/another_memcheck.h
+++ b/src/another_memcheck.h
@@ -24,16 +24,23 @@ namespace SC_AM_Internal {
   };
 
   struct ListNode {
-    ListNode() : next(nullptr), data(nullptr) {}
+    ListNode() : next(nullptr), prev(nullptr), data(nullptr) {}
+
+    static void add_to_list(ListNode *tail, Malloced *data);
+    /// Returns true if removed.
+    static bool remove_from_list(ListNode *head, void *ptr);
 
     ListNode *next;
+    ListNode *prev;
     Malloced *data;
   };
 
+
   struct Stats {
     Stats();
 
-    ListNode *malloced_list;
+    ListNode *malloced_list_head;
+    ListNode *malloced_list_tail;
 
     void initialize();
 
diff --git a/src/another_memcheck_external.cc b/src/another_memcheck_external.cc
index fcd1ed0..dc604f0 100644
--- a/src/another_memcheck_external.cc
+++ b/src/another_memcheck_external.cc
@@ -1,7 +1,5 @@
 // Standard library includes.
 #include <iostream>
-/*#include <cstdlib>*/
-/*#include <cstdio>*/
 
 // Unix includes.
 #include <dlfcn.h>