View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0029777Open CASCADEOCCT:Foundation Classespublic2018-05-15 19:242018-06-23 13:56
ReportermsvAssigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version7.2.0 
Target Version7.4.0Fixed in Version7.4.0 
Summary0029777: Foundation Classes - The methods of moving of one NCollection_Sequence to another are unsafe
DescriptionThe class NCollection_Sequence has methods

Append
Prepend
InserAfter
InsertBefore

that take another sequence and move its content to the current one.

If the sequences have different allocators this causes inconsistent data structure with high probability of application crash.

It is needed to change these methods in a way similar to NCollection_List.
Steps To Reproducecollections n seq
TagsNo tags attached.
Test case numbertests/collections/n/*

Activities

git

2018-05-23 10:19

administrator   ~0076126

Branch CR29777 has been created by msv.

SHA-1: 50378c501b77fbe50ae147cc6331b35664bb42ac


Detailed log of new commits:

Author: msv
Date: Wed May 23 10:18:49 2018 +0300

    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.

kgv

2018-05-23 13:03

developer   ~0076167 

+      // No - this sequence has different memory scope
+      Iterator anIter;
+      prependSeq((const Node *)theSeq.myFirstItem, theIndex + 1);
+      theSeq.Clear();

anIter seems to be an artifact.

   //! InsertBefore theIndex another sequence
   void InsertBefore (const Standard_Integer theIndex,
                      NCollection_Sequence&  theSeq)
   { InsertAfter (theIndex-1, theSeq); }

   //! InsertAfter theIndex theItem
   void InsertAfter  (const Standard_Integer theIndex,
                      NCollection_Sequence&  theSeq)

I think it would be good documenting theSeq behavior as in case of Append().

I never got what is the use case for these weird API like Append(NCollection_Sequence& theSeq) moving the given sequence; from my point of view, this should be done with an extra Boolean flag.

git

2018-05-23 16:22

administrator   ~0076183

Branch CR29777 has been updated by msv.

SHA-1: 5d4b424b47cd834382c02b33022fe091cf98db84


Detailed log of new commits:

Author: msv
Date: Wed May 23 16:22:17 2018 +0300

    # considering remarks

msv

2018-05-23 16:27

developer   ~0076187

The method Append(NCollection_Sequence& theSeq) uses the common symantic with the List class. It is used when the items of a temporary container are to be remembered in the other container. In order to not perform additional allocations the chain is just joined to another list.

kgv

2018-05-24 15:05

developer   ~0076231

Please take the patch.

http://jenkins-test-11.nnov.opencascade.com/view/CR29777-master-KGV/

bugmaster

2018-05-24 17:49

administrator   ~0076246

Combination -
OCCT branch : CR29777 SHA - 5d4b424b47cd834382c02b33022fe091cf98db84
Products branch : master SHA - 300cf879a836fb8a5c4636713070ca9cf544749f
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian70-64:
OCCT
Total CPU difference: 18257.139999999952 / 18243.73999999987 [+0.07%]
Products
Total CPU difference: 7518.890000000061 / 7502.690000000051 [+0.22%]
Windows-64-VC10:
OCCT
Total CPU difference: 18049.112898698488 / 18049.78370299853 [-0.00%]
Products
Total CPU difference: 8266.134187699867 / 8222.282306599882 [+0.53%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2018-05-24 18:28

administrator   ~0076290

Branch CR29777 has been updated forcibly by msv.

SHA-1: 89efe0dfed6ef7ef98249641ba0d3510d111c38d

msv

2018-05-24 18:31

developer   ~0076292

Rebased on current master.

git

2018-06-23 13:56

administrator   ~0076950

Branch CR29777 has been deleted by kgv.

SHA-1: 89efe0dfed6ef7ef98249641ba0d3510d111c38d

Related Changesets

occt: master 395a5977

2018-05-23 07:18:49

msv


Committer: bugmaster Details Diff		 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.		 Affected Issues
0029777
mod - src/NCollection/NCollection_List.hxx Diff File
mod - src/NCollection/NCollection_Sequence.hxx Diff File
mod - src/QANCollection/QANCollection_Test.cxx Diff File
add - tests/collections/n/array1 Diff File
add - tests/collections/n/array2 Diff File
add - tests/collections/n/arrayMove Diff File
add - tests/collections/n/dblmap Diff File
add - tests/collections/n/dmap Diff File
add - tests/collections/n/idmap Diff File
add - tests/collections/n/imap Diff File
add - tests/collections/n/list Diff File
add - tests/collections/n/seq Diff File
add - tests/collections/n/vector Diff File

Issue History

Date Modified Username Field Change
2018-05-15 19:24 msv New Issue
2018-05-15 19:24 msv Assigned To => abv
2018-05-23 10:19 git Note Added: 0076126
2018-05-23 10:20 msv Assigned To abv => msv
2018-05-23 10:20 msv Status new => assigned
2018-05-23 13:03 kgv Note Added: 0076167
2018-05-23 16:22 git Note Added: 0076183
2018-05-23 16:27 msv Note Added: 0076187
2018-05-24 14:24 msv Assigned To msv => kgv
2018-05-24 14:24 msv Status assigned => resolved
2018-05-24 14:24 msv Steps to Reproduce Updated
2018-05-24 15:05 kgv Note Added: 0076231
2018-05-24 15:05 kgv Assigned To kgv => bugmaster
2018-05-24 15:05 kgv Status resolved => reviewed
2018-05-24 17:45 bugmaster Test case number => tests/collections/n/*
2018-05-24 17:49 bugmaster Note Added: 0076246
2018-05-24 17:49 bugmaster Status reviewed => tested
2018-05-24 18:28 git Note Added: 0076290
2018-05-24 18:31 msv Note Added: 0076292
2018-06-14 18:20 bugmaster Changeset attached => occt master 395a5977
2018-06-14 18:20 bugmaster Status tested => verified
2018-06-14 18:20 bugmaster Resolution open => fixed
2018-06-23 13:56 git Note Added: 0076950