MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0029777Open CASCADE[OCCT] OCCT:Foundation Classespublic2018-05-15 19:242018-06-23 13:56
Reportermsv 
Assigned Tobugmaster 
PrioritynormalSeverityminor 
StatusverifiedResolutionfixed 
PlatformOSOS Version
Product Version[OCCT] 7.2.0 
Target Version[OCCT] 7.4.0*Fixed in Version 
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/*
Attached Files

- Relationships

-  Notes
(0076126)
git (administrator)
2018-05-23 10:19

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.
(0076167)
kgv (developer)
2018-05-23 13:03

+      // 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.
(0076183)
git (administrator)
2018-05-23 16:22

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

(0076187)
msv (developer)
2018-05-23 16:27

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.
(0076231)
kgv (developer)
2018-05-24 15:05

Please take the patch.

http://jenkins-test-11.nnov.opencascade.com/view/CR29777-master-KGV/ [^]
(0076246)
bugmaster (administrator)
2018-05-24 17:49

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
(0076290)
git (administrator)
2018-05-24 18:28

Branch CR29777 has been updated forcibly by msv.

SHA-1: 89efe0dfed6ef7ef98249641ba0d3510d111c38d
(0076292)
msv (developer)
2018-05-24 18:31

Rebased on current master.
(0076950)
git (administrator)
2018-06-23 13:56

Branch CR29777 has been deleted by kgv.

SHA-1: 89efe0dfed6ef7ef98249641ba0d3510d111c38d

- Related Changesets
occt: master 395a5977
Timestamp: 2018-05-23 07:18:49
Author: 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.
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-15 19:25 msv Relationship added parent of 0029774
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 View Revisions
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


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker