From: msv Date: Wed, 23 May 2018 07:18:49 +0000 (+0300) Subject: 0029777: Foundation Classes - The methods of moving of one NCollection_Sequence to... X-Git-Url: http://git.dev.opencascade.org/gitweb/?a=commitdiff_plain;h=e24750d195fb1244b8b9974cbe1e2d5de72915f0;p=occt-copy.git 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. --- diff --git a/src/NCollection/NCollection_List.hxx b/src/NCollection/NCollection_List.hxx index ded27c56d9..7025318886 100644 --- a/src/NCollection/NCollection_List.hxx +++ b/src/NCollection/NCollection_List.hxx @@ -272,8 +272,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(); diff --git a/src/NCollection/NCollection_Sequence.hxx b/src/NCollection/NCollection_Sequence.hxx index f50896cf02..e42acd910d 100644 --- a/src/NCollection/NCollection_Sequence.hxx +++ b/src/NCollection/NCollection_Sequence.hxx @@ -174,14 +174,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; } @@ -212,8 +208,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 @@ -223,8 +231,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 @@ -232,7 +252,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); } @@ -242,12 +262,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) { @@ -333,6 +368,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 diff --git a/src/QANCollection/QANCollection_Test.cxx b/src/QANCollection/QANCollection_Test.cxx index f06d11d16e..141cbc6b35 100644 --- a/src/QANCollection/QANCollection_Test.cxx +++ b/src/QANCollection/QANCollection_Test.cxx @@ -24,6 +24,7 @@ #include #include +#include #define ItemType gp_Pnt #define Key1Type Standard_Real @@ -237,6 +238,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(); } @@ -293,6 +311,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(); } @@ -819,8 +853,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); diff --git a/tests/collections/n/array1 b/tests/collections/n/array1 new file mode 100644 index 0000000000..5563982979 --- /dev/null +++ b/tests/collections/n/array1 @@ -0,0 +1,3 @@ +puts "Check NCollection_Array1 functionality" + +QANColTestArray1 1 10 diff --git a/tests/collections/n/array2 b/tests/collections/n/array2 new file mode 100644 index 0000000000..15a4c83016 --- /dev/null +++ b/tests/collections/n/array2 @@ -0,0 +1,3 @@ +puts "Check NCollection_Array2 functionality" + +QANColTestArray2 1 5 1 3 diff --git a/tests/collections/n/arrayMove b/tests/collections/n/arrayMove new file mode 100644 index 0000000000..7b825433d9 --- /dev/null +++ b/tests/collections/n/arrayMove @@ -0,0 +1,4 @@ +puts "Check moving of NCollection_Array1 functionality" + +puts "REQUIRED ALL: Error at item" +QANColTestArrayMove diff --git a/tests/collections/n/dblmap b/tests/collections/n/dblmap new file mode 100644 index 0000000000..2c139135d4 --- /dev/null +++ b/tests/collections/n/dblmap @@ -0,0 +1,3 @@ +puts "Check NCollection_DoubleMap functionality" + +QANColTestDoubleMap diff --git a/tests/collections/n/dmap b/tests/collections/n/dmap new file mode 100644 index 0000000000..f08ba4661a --- /dev/null +++ b/tests/collections/n/dmap @@ -0,0 +1,3 @@ +puts "Check NCollection_DataMap functionality" + +QANColTestDataMap diff --git a/tests/collections/n/idmap b/tests/collections/n/idmap new file mode 100644 index 0000000000..901c064c67 --- /dev/null +++ b/tests/collections/n/idmap @@ -0,0 +1,3 @@ +puts "Check NCollection_IndexedDataMap functionality" + +QANColTestIndexedDataMap diff --git a/tests/collections/n/imap b/tests/collections/n/imap new file mode 100644 index 0000000000..7059f56881 --- /dev/null +++ b/tests/collections/n/imap @@ -0,0 +1,3 @@ +puts "Check NCollection_IndexedMap functionality" + +QANColTestIndexedMap diff --git a/tests/collections/n/list b/tests/collections/n/list new file mode 100644 index 0000000000..44f4973e51 --- /dev/null +++ b/tests/collections/n/list @@ -0,0 +1,3 @@ +puts "Check NCollection_List functionality" + +QANColTestList diff --git a/tests/collections/n/seq b/tests/collections/n/seq new file mode 100644 index 0000000000..9db3fd9137 --- /dev/null +++ b/tests/collections/n/seq @@ -0,0 +1,3 @@ +puts "Check NCollection_Sequence functionality" + +QANColTestSequence diff --git a/tests/collections/n/vector b/tests/collections/n/vector new file mode 100644 index 0000000000..208fe63434 --- /dev/null +++ b/tests/collections/n/vector @@ -0,0 +1,3 @@ +puts "Check NCollection_Vector functionality" + +QANColTestVector