0029777: Foundation Classes - The methods of moving of one NCollection_Sequence to another are unsafe

Make the methods Append, Prepend, InsertBefore and InsertAfter, which take another sequence as an argument, copying the sequence instead of joining if the allocators are different.

Add test cases for collection classes.

src/NCollection/NCollection_List.hxx

@@ -274,8 +274,6 @@ public:
     else
     {
       // No - this list has different memory scope
-      Standard_NoSuchObject_Raise_if (!theIter.More(), "NCollection_List::InsertAfter");
-
       Iterator anIter;
       anIter.myPrevious = theIter.myCurrent;
       anIter.myCurrent = theIter.myCurrent->Next();

src/NCollection/NCollection_Sequence.hxx

@@ -176,14 +176,10 @@ public:
   //! This method does not change the internal allocator.
   NCollection_Sequence& Assign (const NCollection_Sequence& theOther)
   { 
-    if (this == &theOther) 
-      return *this;
-    Clear ();
-    Node * pCur = (Node *) theOther.myFirstItem;
-    while (pCur) {
-      Node* pNew = new (this->myAllocator) Node (pCur->Value());
-      PAppend (pNew);
-      pCur = (Node *) pCur->Next();
+    if (this != &theOther)
+    {
+      Clear();
+      appendSeq((const Node *)theOther.myFirstItem);
     }
     return * this;
   }
@@ -214,8 +210,20 @@ public:
   //! Append another sequence (making it empty)
   void Append (NCollection_Sequence& theSeq)
   {
-    if (myFirstItem == theSeq.myFirstItem) Assign (theSeq);
-    PAppend (theSeq);
+    if (this == &theSeq || theSeq.IsEmpty())
+      return;
+    if (this->myAllocator == theSeq.myAllocator)
+    {
+      // Then we take the sequence and glue it to our end - 
+      // deallocation will bring no problem
+      PAppend(theSeq);
+    }
+    else
+    {
+      // No - this sequence has different memory scope
+      appendSeq((const Node *)theSeq.myFirstItem);
+      theSeq.Clear();
+    }
   }
 
   //! Prepend one item
@@ -225,8 +233,20 @@ public:
   //! Prepend another sequence (making it empty)
   void Prepend (NCollection_Sequence& theSeq)
   {
-    if (myFirstItem == theSeq.myFirstItem) Assign (theSeq);
-    PPrepend (theSeq);
+    if (this == &theSeq || theSeq.IsEmpty())
+      return;
+    if (this->myAllocator == theSeq.myAllocator)
+    {
+      // Then we take the sequence and glue it to our head - 
+      // deallocation will bring no problem
+      PPrepend(theSeq);
+    }
+    else
+    {
+      // No - this sequence has different memory scope
+      prependSeq((const Node *)theSeq.myFirstItem, 1);
+      theSeq.Clear();
+    }
   }
 
   //! InsertBefore theIndex theItem
@@ -234,7 +254,7 @@ public:
                      const TheItemType&     theItem)
   { InsertAfter (theIndex-1, theItem); }
 
-  //! InsertBefore theIndex another sequence
+  //! InsertBefore theIndex another sequence (making it empty)
   void InsertBefore (const Standard_Integer theIndex,
                      NCollection_Sequence&  theSeq)
   { InsertAfter (theIndex-1, theSeq); }
@@ -244,12 +264,27 @@ public:
                      const TheItemType&     theItem)
   { PInsertAfter (thePosition, new (this->myAllocator) Node (theItem)); }
 
-  //! InsertAfter theIndex theItem
+  //! InsertAfter theIndex another sequence (making it empty)
   void InsertAfter  (const Standard_Integer theIndex,
                      NCollection_Sequence&  theSeq)
-  { PInsertAfter (theIndex, theSeq); }
+  {
+    if (this == &theSeq || theSeq.IsEmpty())
+      return;
+    if (this->myAllocator == theSeq.myAllocator)
+    {
+      // Then we take the list and glue it to our head - 
+      // deallocation will bring no problem
+      PInsertAfter(theIndex, theSeq);
+    }
+    else
+    {
+      // No - this sequence has different memory scope
+      prependSeq((const Node *)theSeq.myFirstItem, theIndex + 1);
+      theSeq.Clear();
+    }
+  }
 
-  //! InsertAfter theIndex another sequence
+  //! InsertAfter theIndex theItem
   void InsertAfter (const Standard_Integer  theIndex, 
                     const TheItemType&      theItem)
   {
@@ -335,6 +370,31 @@ public:
   // ---------- FRIEND CLASSES ------------
   friend class Iterator;
 
+  // ----------- PRIVATE METHODS -----------
+
+  //! append the sequence headed by the given Node
+  void appendSeq(const Node * pCur)
+  {
+    while (pCur)
+    {
+      Node* pNew = new (this->myAllocator) Node(pCur->Value());
+      PAppend(pNew);
+      pCur = (const Node *)pCur->Next();
+    }
+  }
+
+  //! insert the sequence headed by the given Node before the item with the given index
+  void prependSeq(const Node * pCur, Standard_Integer ind)
+  {
+    ind--;
+    while (pCur)
+    {
+      Node* pNew = new (this->myAllocator) Node(pCur->Value());
+      PInsertAfter(ind++, pNew);
+      pCur = (const Node *)pCur->Next();
+    }
+  }
+
 };
 
 #endif

src/QANCollection/QANCollection_Test.cxx

@@ -25,6 +25,7 @@
 #include <Standard_Overflow.hxx>
 
 #include <NCollection_Vector.hxx>
+#include <NCollection_IncAllocator.hxx>
 
 #define ItemType gp_Pnt
 #define Key1Type Standard_Real
@@ -238,6 +239,23 @@ static void TestList (QANCollection_ListFunc&     theL)
   // Assign
   AssignCollection (theL, aL);
 
+  // Different allocators
+  {
+    // The joining of list having different allocator can cause memory error
+    // if the fact of different allocator is not taken into account.
+    Handle(NCollection_IncAllocator) anAlloc = new NCollection_IncAllocator;
+    QANCollection_ListFunc aS2(anAlloc);
+    aS2.Append(anItem);
+    theL.Prepend(aS2);
+    aS2.Append(anItem);
+    theL.Append(aS2);
+    aS2.Append(anItem);
+    QANCollection_ListFunc::Iterator anIter(theL);
+    theL.InsertBefore(aS2, anIter);
+    aS2.Append(anItem);
+    theL.InsertAfter(aS2, anIter);
+  }
+
   // Clear
   aL.Clear();
 }
@@ -294,6 +312,22 @@ static void TestSequence (QANCollection_SequenceFunc& theS)
   // Assign
   AssignCollection (theS, aS);
 
+  // Different allocators
+  {
+    // The joining of sequence having different allocator can cause memory error
+    // if the fact of different allocator is not taken into account.
+    Handle(NCollection_IncAllocator) anAlloc = new NCollection_IncAllocator;
+    QANCollection_SequenceFunc aS2(anAlloc);
+    aS2.Append(anItem);
+    theS.Prepend(aS2);
+    aS2.Append(anItem);
+    theS.Append(aS2);
+    aS2.Append(anItem);
+    theS.InsertBefore(1, aS2);
+    aS2.Append(anItem);
+    theS.InsertAfter(1, aS2);
+  }
+
   // Clear
   aS.Clear();
 }
@@ -1113,8 +1147,10 @@ void QANCollection::CommandsTest(Draw_Interpretor& theCommands) {
   const char *group = "QANCollection";
 
   // from agvCollTest/src/CollectionEXE/FuncTestEXE.cxx
-  theCommands.Add("QANColTestArray1",         "QANColTestArray1",         __FILE__, QANColTestArray1,         group);  
-  theCommands.Add("QANColTestArray2",         "QANColTestArray2",         __FILE__, QANColTestArray2,         group);  
+  theCommands.Add("QANColTestArray1",         "QANColTestArray1 Lower Upper",
+    __FILE__, QANColTestArray1, group);
+  theCommands.Add("QANColTestArray2",         "QANColTestArray2 LowerRow UpperRow LowerCol UpperCol",
+    __FILE__, QANColTestArray2, group);  
   theCommands.Add("QANColTestMap",            "QANColTestMap",            __FILE__, QANColTestMap,            group);  
   theCommands.Add("QANColTestDataMap",        "QANColTestDataMap",        __FILE__, QANColTestDataMap,        group);  
   theCommands.Add("QANColTestDoubleMap",      "QANColTestDoubleMap",      __FILE__, QANColTestDoubleMap,      group);  

tests/collections/n/array1

@@ -0,0 +1,3 @@
+puts "Check NCollection_Array1 functionality"
+
+QANColTestArray1 1 10

tests/collections/n/array2

@@ -0,0 +1,3 @@
+puts "Check NCollection_Array2 functionality"
+
+QANColTestArray2 1 5 1 3

tests/collections/n/arrayMove

@@ -0,0 +1,4 @@
+puts "Check moving of NCollection_Array1 functionality"
+
+puts "REQUIRED ALL: Error at item"
+QANColTestArrayMove

tests/collections/n/dblmap

@@ -0,0 +1,3 @@
+puts "Check NCollection_DoubleMap functionality"
+
+QANColTestDoubleMap

tests/collections/n/dmap

@@ -0,0 +1,3 @@
+puts "Check NCollection_DataMap functionality"
+
+QANColTestDataMap

tests/collections/n/idmap

@@ -0,0 +1,3 @@
+puts "Check NCollection_IndexedDataMap functionality"
+
+QANColTestIndexedDataMap

tests/collections/n/imap

@@ -0,0 +1,3 @@
+puts "Check NCollection_IndexedMap functionality"
+
+QANColTestIndexedMap

tests/collections/n/list

@@ -0,0 +1,3 @@
+puts "Check NCollection_List functionality"
+
+QANColTestList

tests/collections/n/seq

@@ -0,0 +1,3 @@
+puts "Check NCollection_Sequence functionality"
+
+QANColTestSequence

tests/collections/n/vector

@@ -0,0 +1,3 @@
+puts "Check NCollection_Vector functionality"
+
+QANColTestVector