From 53e05d096c52042274ab92f05c0e4b367b1f6d31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Constantin=20F=C3=BCrst?= <c@fuersten.info>
Date: Wed, 10 Jan 2024 12:41:32 +0100
Subject: [PATCH] add even more comments and remove an old code line that
 modified the reference counter of cache data from the outside

---
 offloading-cacher/cache-data.hpp |  3 ++-
 offloading-cacher/cache.hpp      | 41 ++++++++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/offloading-cacher/cache-data.hpp b/offloading-cacher/cache-data.hpp
index 4de6138..fe02c90 100644
--- a/offloading-cacher/cache-data.hpp
+++ b/offloading-cacher/cache-data.hpp
@@ -66,7 +66,8 @@ namespace dsacache {
 
         // returns the cache data location for this
         // instance which is valid as long as the
-        // instance is alive
+        // instance is alive - !!! this may also
+        // yield a nullptr !!!
         uint8_t* GetDataLocation() const;
     };
 }
diff --git a/offloading-cacher/cache.hpp b/offloading-cacher/cache.hpp
index f3ef90d..8fd8362 100644
--- a/offloading-cacher/cache.hpp
+++ b/offloading-cacher/cache.hpp
@@ -255,23 +255,45 @@ void dsacache::Cache::GetCacheNode(uint8_t* src, const size_t size, int* OUT_DST
 inline void dsacache::Cache::Flush(const int node) {
     std::cout << "[-] Flushing Cache for " << (node == -1 ? "all nodes" : "node " + std::to_string(node)) << std::endl;
 
+    // this lambda is used because below we have two code paths that
+    // flush nodes, either one single or all successively
+
     const auto FlushNode = [](std::unordered_map<uint8_t*,CacheData>& map) {
+        // begin at the front of the map
+
         auto it = map.begin();
 
+        // loop until we reach the end of the map
+
         while (it != map.end()) {
+            // if the iterator points to an inactive element
+            // then we may erase it
+
             if (it->second.Active() == false) {
+                // erase the iterator from the map
+
                 map.erase(it);
+
+                // as the erasure invalidated out iterator
+                // we must start at the beginning again
+
                 it = map.begin();
             }
             else {
+                // if element is active just move over to the next one
+
                 it++;
             }
         }
     };
 
     {
+        // we require exclusive lock as we modify the cache state
+
         std::unique_lock<std::shared_mutex> lock(cache_mutex_);
 
+        // node == -1 means that cache on all nodes should be flushed
+
         if (node == -1) {
             for (auto& nc : cache_state_) {
                 FlushNode(nc.second);
@@ -290,21 +312,36 @@ std::unique_ptr<dsacache::CacheData> dsacache::Cache::GetFromCache(uint8_t* src,
     // from marking the element we may find as unused and
     // clearing it
 
+    // lock the cache state in shared-mode because we read
+
     std::shared_lock<std::shared_mutex> lock(cache_mutex_);
 
+    // search for the data in our cache state structure at the given node
+
     const auto search = cache_state_[dst_node].find(src);
 
+    // if the data is in our structure we continue
+
     if (search != cache_state_[dst_node].end()) {
-        if (search->second.size_ == size) {
-            search->second.active_->store(true);
 
+        // now check whether the sizes match
+        // TODO: second.size_ >= size would also work
+
+        if (search->second.size_ == size) {
             std::cout << "[+] Found Cached version for 0x" << std::hex << (uint64_t)src << std::dec << std::endl;
 
+            // return a unique copy of the entry which uses the object
+            // lifetime and destructor to safely handle deallocation
+
             return std::move(std::make_unique<CacheData>(search->second));
         }
         else {
             std::cout << "[!] Found Cached version with size missmatch for 0x" << std::hex << (uint64_t)src << std::dec << std::endl;
 
+            // if the sizes missmatch then we clear the current entry from cache
+            // which will cause its deletion only after the last possible outside
+            // reference is also destroyed
+
             cache_state_[dst_node].erase(search);
         }
     }