Cleaned up dynamic memory management, explicit pointer zeroing from destroy()

libuavcan/include/uavcan/internal/map.hpp

@@ -46,6 +46,26 @@ class Map : Noncopyable
             IsDynamicallyAllocatable<KVGroup>::check();
         }
 
+        static KVGroup* instantiate(IAllocator* allocator)
+        {
+            assert(allocator);
+            void* const praw = allocator->allocate(sizeof(KVGroup));
+            if (praw == NULL)
+                return NULL;
+            return new (praw) KVGroup();
+        }
+
+        static void destroy(KVGroup*& obj, IAllocator* allocator)
+        {
+            assert(allocator);
+            if (obj != NULL)
+            {
+                obj->~KVGroup();
+                allocator->deallocate(obj);
+                obj = NULL;
+            }
+        }
+
         KVPair* find(const Key& key)
         {
             for (int i = 0; i < NUM_KV; i++)
@@ -140,8 +160,7 @@ class Map : Noncopyable
             if (remove_this)
             {
                 list_.remove(p);
-                p->~KVGroup();
-                allocator_->deallocate(p);
+                KVGroup::destroy(p, allocator_);
             }
             p = next;
         }
@@ -186,14 +205,11 @@ public:
             return &kv->value;
         }
 
-        void* const praw = allocator_->allocate(sizeof(KVGroup));
-        if (praw == NULL)
+        KVGroup* const kvg = KVGroup::instantiate(allocator_);
+        if (kvg == NULL)
             return NULL;
-
-        KVGroup* const kvg = new (praw) KVGroup();
-        assert(kvg);
-        kvg->kvs[0] = KVPair(key, value);
         list_.insert(kvg);
+        kvg->kvs[0] = KVPair(key, value);
         return &kvg->kvs[0].value;
     }
 

libuavcan/include/uavcan/internal/transport/can_io.hpp

@@ -53,6 +53,8 @@ public:
             IsDynamicallyAllocatable<Entry>::check();
         }
 
+        static void destroy(Entry*& obj, IAllocator* allocator);
+
         bool isExpired(uint64_t monotonic_timestamp) const { return monotonic_timestamp > monotonic_deadline; }
 
         bool qosHigherThan(const CanFrame& rhs_frame, Qos rhs_qos) const;
@@ -104,7 +106,7 @@ public:
     void push(const CanFrame& frame, uint64_t monotonic_tx_deadline, Qos qos);
 
     Entry* peek();               // Modifier
-    void remove(Entry* entry);
+    void remove(Entry*& entry);
 
     bool topPriorityHigherOrEqual(const CanFrame& rhs_frame) const;
 

libuavcan/src/transport/can_io.cpp

@@ -5,6 +5,7 @@
 
 #include <algorithm>
 #include <sstream>
+#include <cassert>
 #include <limits>
 #include <uavcan/internal/transport/can_io.hpp>
 #include <uavcan/internal/debug.hpp>
@@ -25,6 +26,17 @@ std::string CanRxFrame::toString(StringRepresentation mode) const
 /*
  * CanTxQueue::Entry
  */
+void CanTxQueue::Entry::destroy(Entry*& obj, IAllocator* allocator)
+{
+    assert(allocator);
+    if (obj != NULL)
+    {
+        obj->~Entry();
+        allocator->deallocate(obj);
+        obj = NULL;
+    }
+}
+
 bool CanTxQueue::Entry::qosHigherThan(const CanFrame& rhs_frame, Qos rhs_qos) const
 {
     if (qos != rhs_qos)
@@ -161,7 +173,7 @@ CanTxQueue::Entry* CanTxQueue::peek()
     return NULL;
 }
 
-void CanTxQueue::remove(Entry* entry)
+void CanTxQueue::remove(Entry*& entry)
 {
     if (entry == NULL)
     {
@@ -169,8 +181,7 @@ void CanTxQueue::remove(Entry* entry)
         return;
     }
     queue_.remove(entry);
-    entry->~Entry();
-    allocator_->deallocate(entry);
+    Entry::destroy(entry, allocator_);
 }
 
 bool CanTxQueue::topPriorityHigherOrEqual(const CanFrame& rhs_frame) const
@@ -207,7 +218,7 @@ int CanIOManager::sendToIface(int iface_index, const CanFrame& frame, uint64_t m
 int CanIOManager::sendFromTxQueue(int iface_index)
 {
     assert(iface_index >= 0 && iface_index < MAX_IFACES);
-    CanTxQueue::Entry* const entry = tx_queues_[iface_index].peek();
+    CanTxQueue::Entry* entry = tx_queues_[iface_index].peek();
     if (entry == NULL)
         return 0;
     const int res = sendToIface(iface_index, entry->frame, entry->monotonic_deadline);

libuavcan/test/transport/can/tx_queue.cpp

@@ -190,10 +190,19 @@ TEST(CanTxQueue, TxQueue)
     EXPECT_TRUE(isInQueue(queue, f4));
     EXPECT_EQ(f4, queue.peek()->frame);
 
-    queue.remove(queue.peek());
+    CanTxQueue::Entry* entry = queue.peek();
+    EXPECT_TRUE(entry);
+    queue.remove(entry);
+    EXPECT_FALSE(entry);
+
     EXPECT_FALSE(isInQueue(queue, f4));
     EXPECT_TRUE(isInQueue(queue, f5));
-    queue.remove(queue.peek());
+
+    entry = queue.peek();
+    EXPECT_TRUE(entry);
+    queue.remove(entry);
+    EXPECT_FALSE(entry);
+
     EXPECT_FALSE(isInQueue(queue, f5));
 
     EXPECT_EQ(0, getQueueLength(queue));           // Final state checks