From 62befa2d029cb4ab998278da17f0e269edb4c63f Mon Sep 17 00:00:00 2001
From: Terraneo Federico <fede.tft@miosix.org>
Date: Fri, 17 Mar 2023 23:36:47 +0100
Subject: [PATCH] IntrusiveList: add error check and testsuite, fix iterator --

---
 miosix/Makefile                  |   1 +
 miosix/kernel/intrusive.cpp      | 236 +++++++++++++++++++++++++++++++
 miosix/kernel/intrusive.h        |  96 ++++++++++---
 miosix/kernel/timeconversion.cpp |   2 +-
 4 files changed, 314 insertions(+), 21 deletions(-)
 create mode 100644 miosix/kernel/intrusive.cpp

diff --git a/miosix/Makefile b/miosix/Makefile
index 07a6266b..39671cd7 100644
--- a/miosix/Makefile
+++ b/miosix/Makefile
@@ -19,6 +19,7 @@ kernel/elf_program.cpp                                                     \
 kernel/process.cpp                                                         \
 kernel/process_pool.cpp                                                    \
 kernel/timeconversion.cpp                                                  \
+kernel/intrusive.cpp                                                       \
 kernel/SystemMap.cpp                                                       \
 kernel/cpu_time_counter.cpp                                                \
 kernel/scheduler/priority/priority_scheduler.cpp                           \
diff --git a/miosix/kernel/intrusive.cpp b/miosix/kernel/intrusive.cpp
new file mode 100644
index 00000000..43d00798
--- /dev/null
+++ b/miosix/kernel/intrusive.cpp
@@ -0,0 +1,236 @@
+
+#ifndef TEST_ALGORITHM
+
+#include "intrusive.h"
+
+#else //TEST_ALGORITHM
+
+#include <iostream>
+#include <cassert>
+
+// Unused stubs as the test code only tests IntrusiveList
+inline int atomicSwap(volatile int*, int) { return 0; }
+void *atomicFetchAndIncrement(void *const volatile*, int, int) { return nullptr; }
+
+//C++ glassbox testing trick
+#define private public
+#define protected public
+#include "intrusive.h"
+#undef private
+#undef public
+
+using namespace std;
+using namespace miosix;
+
+#endif //TEST_ALGORITHM
+
+
+
+
+
+//Testsuite for IntrusiveList. Compile with:
+//g++ -DTEST_ALGORITHM -DINTRUSIVE_LIST_ERROR_CHECK -fsanitize=address -m32
+//    -std=c++14 -Wall -O2 -o test intrusive.cpp; ./test
+#ifdef TEST_ALGORITHM
+
+void emptyCheck(IntrusiveListItem& x)
+{
+    //Glass box check
+    assert(x.next==nullptr); assert(x.prev==nullptr);
+}
+
+void emptyCheck(IntrusiveList<IntrusiveListItem>& list)
+{
+    //Black box check
+    assert(list.empty());
+    assert(list.begin()==list.end());
+    //Glass box check
+    assert(list.head==nullptr);
+    assert(list.tail==nullptr);
+}
+
+void oneItemCheck(IntrusiveList<IntrusiveListItem>& list, IntrusiveListItem& a)
+{
+    IntrusiveList<IntrusiveListItem>::iterator it;
+    //Black box check
+    assert(list.empty()==false);
+    assert(list.front()==&a);
+    assert(list.back()==&a);
+    assert(list.begin()!=list.end());
+    assert(*list.begin()==&a);
+    assert(++list.begin()==list.end());
+    it=list.begin(); it++; assert(it==list.end());
+    assert(--list.end()==list.begin());
+    it=list.end(); it--; assert(it==list.begin());
+    //Glass box check
+    assert(list.head==&a);
+    assert(list.tail==&a);
+    assert(a.prev==nullptr);
+    assert(a.next==nullptr);
+}
+
+void twoItemCheck(IntrusiveList<IntrusiveListItem>& list, IntrusiveListItem& a,
+                  IntrusiveListItem& b)
+{
+    IntrusiveList<IntrusiveListItem>::iterator it;
+    //Black box check
+    assert(list.empty()==false);
+    assert(list.front()==&a);
+    assert(list.back()==&b);
+    assert(list.begin()!=list.end());
+    it=list.begin();
+    assert(*it++==&a);
+    assert(*it++==&b);
+    assert(it==list.end());
+    it=list.begin();
+    assert(*it==&a);
+    ++it;
+    assert(*it==&b);
+    ++it;
+    assert(it==list.end());
+    it=list.end();
+    it--;
+    assert(*it==&b);
+    it--;
+    assert(*it==&a);
+    assert(it==list.begin());
+    it=list.end();
+    assert(*--it==&b);
+    assert(*--it==&a);
+    assert(it==list.begin());
+    //Glass box check
+    assert(list.head==&a);
+    assert(list.tail==&b);
+    assert(a.prev==nullptr);
+    assert(a.next==&b);
+    assert(b.prev==&a);
+    assert(b.next==nullptr);
+}
+
+void threeItemCheck(IntrusiveList<IntrusiveListItem>& list, IntrusiveListItem& a,
+                  IntrusiveListItem& b, IntrusiveListItem& c)
+{
+    IntrusiveList<IntrusiveListItem>::iterator it;
+    //Black box check
+    assert(list.empty()==false);
+    assert(list.front()==&a);
+    assert(list.back()==&c);
+    assert(list.begin()!=list.end());
+    it=list.begin();
+    assert(*it++==&a);
+    assert(*it++==&b);
+    assert(*it++==&c);
+    assert(it==list.end());
+    it=list.begin();
+    assert(*it==&a);
+    ++it;
+    assert(*it==&b);
+    ++it;
+    assert(*it==&c);
+    ++it;
+    assert(it==list.end());
+    it=list.end();
+    it--;
+    assert(*it==&c);
+    it--;
+    assert(*it==&b);
+    it--;
+    assert(*it==&a);
+    assert(it==list.begin());
+    it=list.end();
+    assert(*--it==&c);
+    assert(*--it==&b);
+    assert(*--it==&a);
+    assert(it==list.begin());
+    //Glass box check
+    assert(list.head==&a);
+    assert(list.tail==&c);
+    assert(a.prev==nullptr);
+    assert(a.next==&b);
+    assert(b.prev==&a);
+    assert(b.next==&c);
+    assert(c.prev==&b);
+    assert(c.next==nullptr);
+}
+
+int main()
+{
+    IntrusiveListItem a,b,c;
+    IntrusiveList<IntrusiveListItem> list;
+    emptyCheck(a);
+    emptyCheck(b);
+    emptyCheck(c);
+    emptyCheck(list);
+
+    //
+    // Testing push_back / pop_back
+    //
+    list.push_back(&a);
+    oneItemCheck(list,a);
+    list.push_back(&b);
+    twoItemCheck(list,a,b);
+    list.pop_back();
+    oneItemCheck(list,a);
+    emptyCheck(b);
+    list.pop_back();
+    emptyCheck(list);
+    emptyCheck(a);
+
+    //
+    // Testing push_front / pop_front
+    //
+    list.push_front(&a);
+    oneItemCheck(list,a);
+    list.push_front(&b);
+    twoItemCheck(list,b,a);
+    list.pop_front();
+    oneItemCheck(list,a);
+    emptyCheck(b);
+    list.pop_front();
+    emptyCheck(list);
+    emptyCheck(a);
+
+    //
+    // Testing insert / erase
+    //
+    list.insert(list.end(),&a);
+    oneItemCheck(list,a);
+    list.insert(list.end(),&b);
+    twoItemCheck(list,a,b);
+    list.erase(++list.begin()); //Erase second item first
+    oneItemCheck(list,a);
+    emptyCheck(b);
+    list.erase(list.begin());   //Erase only item
+    emptyCheck(list);
+    emptyCheck(a);
+    list.insert(list.begin(),&a);
+    oneItemCheck(list,a);
+    list.insert(list.begin(),&b);
+    twoItemCheck(list,b,a);
+    list.erase(list.begin());   //Erase first item first
+    oneItemCheck(list,a);
+    emptyCheck(b);
+    list.erase(list.begin());   //Erase only item
+    emptyCheck(list);
+    emptyCheck(a);
+    list.insert(list.end(),&a);
+    oneItemCheck(list,a);
+    list.insert(list.end(),&c);
+    twoItemCheck(list,a,c);
+    list.insert(++list.begin(),&b); //Insert in the middle
+    threeItemCheck(list,a,b,c);
+    list.erase(++list.begin());     //Erase in the middle
+    twoItemCheck(list,a,c);
+    emptyCheck(b);
+    list.erase(list.begin());
+    oneItemCheck(list,c);
+    emptyCheck(a);
+    list.erase(list.begin());
+    emptyCheck(list);
+    emptyCheck(c);
+
+    cout<<"Test passed"<<endl;
+    return 0;
+}
+
+#endif //TEST_ALGORITHM
diff --git a/miosix/kernel/intrusive.h b/miosix/kernel/intrusive.h
index f1e3732a..755bc393 100755
--- a/miosix/kernel/intrusive.h
+++ b/miosix/kernel/intrusive.h
@@ -31,7 +31,13 @@
 #include <cstddef>
 #include <cassert>
 #include <type_traits>
+#ifndef TEST_ALGORITHM
 #include "interfaces/atomic_ops.h"
+#include "error.h"
+#endif //TEST_ALGORITHM
+
+//Only enable when testing code that uses IntrusiveList
+//#define INTRUSIVE_LIST_ERROR_CHECK
 
 namespace miosix {
 
@@ -669,22 +675,52 @@ public:
     class iterator
     {
     public:
-        iterator() : cur(nullptr) {}
+        iterator() : list(nullptr), cur(nullptr) {}
+
+        T* operator*()
+        {
+            #ifdef INTRUSIVE_LIST_ERROR_CHECK
+            if(list==nullptr || cur==nullptr) IntrusiveList<T>::fail();
+            #endif //INTRUSIVE_LIST_ERROR_CHECK
+            return static_cast<T*>(cur);
+        }
 
-        T* operator*() { return static_cast<T*>(cur); }
+        iterator operator++()
+        {
+            #ifdef INTRUSIVE_LIST_ERROR_CHECK
+            if(list==nullptr || cur==nullptr) IntrusiveList<T>::fail();
+            #endif //INTRUSIVE_LIST_ERROR_CHECK
+            cur=cur->next; return *this;
+        }
+
+        iterator operator--()
+        {
+            #ifdef INTRUSIVE_LIST_ERROR_CHECK
+            if(list==nullptr || list->empty()) IntrusiveList<T>::fail();
+            #endif //INTRUSIVE_LIST_ERROR_CHECK
+            if(cur!=nullptr) cur=cur->prev;
+            else cur=list->tail; //Special case: decrementing end()
+            return *this;
+        }
 
-        iterator operator++() { cur=cur->next; return *this; }
-        iterator operator--() { cur=cur->prev; return *this; }
         iterator operator++(int)
         {
+            #ifdef INTRUSIVE_LIST_ERROR_CHECK
+            if(list==nullptr || cur==nullptr) IntrusiveList<T>::fail();
+            #endif //INTRUSIVE_LIST_ERROR_CHECK
             iterator result=*this;
             cur=cur->next;
             return result;
         }
+
         iterator operator--(int)
         {
+            #ifdef INTRUSIVE_LIST_ERROR_CHECK
+            if(list==nullptr || list->empty()) IntrusiveList<T>::fail();
+            #endif //INTRUSIVE_LIST_ERROR_CHECK
             iterator result=*this;
-            cur=cur->prev;
+            if(cur!=nullptr) cur=cur->prev;
+            else cur=list->tail; //Special case: decrementing end()
             return result;
         }
 
@@ -692,8 +728,10 @@ public:
         bool operator!=(const iterator& rhs) { return cur!=rhs.cur; }
 
     private:
-        iterator(IntrusiveListItem *cur) : cur(cur) {}
+        iterator(IntrusiveList<T> *list, IntrusiveListItem *cur)
+            : list(list), cur(cur) {}
 
+        IntrusiveList<T> *list;
         IntrusiveListItem *cur;
 
         friend class IntrusiveList<T>;
@@ -719,8 +757,8 @@ public:
     void push_back(T *item)
     {
         #ifdef INTRUSIVE_LIST_ERROR_CHECK
-        if(head!=nullptr ^ tail!=nullptr) fail();
-        if(head==tail && !(head->prev==nullptr && head->next==nullptr)) fail();
+        if((head!=nullptr) ^ (tail!=nullptr)) fail();
+        if(!empty() && head==tail && (head->prev || head->next)) fail();
         if(item->prev!=nullptr || item->next!=nullptr) fail();
         #endif //INTRUSIVE_LIST_ERROR_CHECK
         if(empty()) head=item;
@@ -738,7 +776,7 @@ public:
     {
         #ifdef INTRUSIVE_LIST_ERROR_CHECK
         if(head==nullptr || tail==nullptr) fail();
-        if(head==tail && !(head->prev==nullptr && head->next==nullptr)) fail();
+        if(!empty() && head==tail && (head->prev || head->next)) fail();
         #endif //INTRUSIVE_LIST_ERROR_CHECK
         IntrusiveListItem *removedItem=tail;
         tail=removedItem->prev;
@@ -756,8 +794,8 @@ public:
     void push_front(T *item)
     {
         #ifdef INTRUSIVE_LIST_ERROR_CHECK
-        if(head!=nullptr ^ tail!=nullptr) fail();
-        if(head==tail && !(head->prev==nullptr && head->next==nullptr)) fail();
+        if((head!=nullptr) ^ (tail!=nullptr)) fail();
+        if(!empty() && head==tail && (head->prev || head->next)) fail();
         if(item->prev!=nullptr || item->next!=nullptr) fail();
         #endif //INTRUSIVE_LIST_ERROR_CHECK
         if(empty()) tail=item;
@@ -775,7 +813,7 @@ public:
     {
         #ifdef INTRUSIVE_LIST_ERROR_CHECK
         if(head==nullptr || tail==nullptr) fail();
-        if(head==tail && !(head->prev==nullptr && head->next==nullptr)) fail();
+        if(!empty() && head==tail && (head->prev || head->next)) fail();
         #endif //INTRUSIVE_LIST_ERROR_CHECK
         IntrusiveListItem *removedItem=head;
         head=removedItem->next;
@@ -793,10 +831,11 @@ public:
      */
     void insert(iterator it, T *item)
     {
-        IntrusiveListItem *cur=*it;
+        IntrusiveListItem *cur=it.cur; //Safe even if it==end(), cur==nullptr
         #ifdef INTRUSIVE_LIST_ERROR_CHECK
-        if(head!=nullptr ^ tail!=nullptr) fail();
-        if(head==tail && !(head->prev==nullptr && head->next==nullptr)) fail();
+        if((head!=nullptr) ^ (tail!=nullptr)) fail();
+        if(!empty() && head==tail && (head->prev || head->next)) fail();
+        if(it.list!=this) fail();
         if(cur!=nullptr)
         {
             if(cur->prev==nullptr && cur!=head) fail();
@@ -824,10 +863,11 @@ public:
      */
     iterator erase(iterator it)
     {
-        IntrusiveListItem *cur=*it;
+        IntrusiveListItem *cur=it.cur; //Safe even if it==end(), cur==nullptr
         #ifdef INTRUSIVE_LIST_ERROR_CHECK
         if(head==nullptr || tail==nullptr) fail();
-        if(head==tail && !(head->prev==nullptr && head->next==nullptr)) fail();
+        if(!empty() && head==tail && (head->prev || head->next)) fail();
+        if(it.list!=this) fail();
         if(cur==nullptr) fail();
         if(cur->prev==nullptr && cur!=head) fail();
         if(cur->next==nullptr && cur!=tail) fail();
@@ -836,7 +876,7 @@ public:
         else head=cur->next;
         if(cur->next!=nullptr) cur->next->prev=cur->prev;
         else tail=cur->prev;
-        iterator result(cur->next);
+        iterator result(this,cur->next);
         cur->prev=nullptr;
         cur->next=nullptr;
         return result;
@@ -845,12 +885,12 @@ public:
     /**
      * \return an iterator to the first item
      */
-    iterator begin() { return iterator(head); }
+    iterator begin() { return iterator(this,head); }
     
     /**
      * \return an iterator to the last item
      */
-    iterator end() { return iterator(nullptr); }
+    iterator end() { return iterator(this,nullptr); }
     
     /**
      * \return a pointer to the first item. List must not be empty
@@ -868,8 +908,24 @@ public:
     bool empty() const { return head==nullptr; }
 
 private:
+    #ifdef INTRUSIVE_LIST_ERROR_CHECK
+    static void fail();
+    #endif //INTRUSIVE_LIST_ERROR_CHECK
+
     IntrusiveListItem *head;
     IntrusiveListItem *tail;
 };
 
+#ifdef INTRUSIVE_LIST_ERROR_CHECK
+template<typename T>
+void IntrusiveList<T>::fail()
+{
+    #ifndef TEST_ALGORITHM
+    errorHandler(UNEXPECTED);
+    #else //TEST_ALGORITHM
+    assert(false);
+    #endif //TEST_ALGORITHM
+}
+#endif //INTRUSIVE_LIST_ERROR_CHECK
+
 } //namespace miosix
diff --git a/miosix/kernel/timeconversion.cpp b/miosix/kernel/timeconversion.cpp
index 79d9109e..228074c1 100644
--- a/miosix/kernel/timeconversion.cpp
+++ b/miosix/kernel/timeconversion.cpp
@@ -449,7 +449,7 @@ TimeConversionFactor TimeConversion::floatToFactor(float x) noexcept
 } //namespace miosix
 
 //Testsuite for multiplication algorithm and factor computation. Compile with:
-// g++ -std=c++11 -O2 -DTEST_ALGORITHM -o test timeconversion.cpp; ./test
+// g++ -std=c++14 -O2 -DTEST_ALGORITHM -o test timeconversion.cpp; ./test
 #ifdef TEST_ALGORITHM
 
 using namespace std;
-- 
GitLab