View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0024863Open CASCADEOCCT:Codingpublic2014-04-21 15:302014-11-11 12:57
ReporterskiAssigned Toapv 
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version6.7.1 
Target Version6.8.0Fixed in Version6.8.0 
Summary0024863: CLang warnings -Wint-to-void-pointer-cast
DescriptionCLang on Mac OS X 10.9 reports warning -Wint-to-void-pointer-cast:
Draw_VariableCommands.cxx:803:15: warning: cast to 'void *' from smaller integer type 'Standard_Integer' (aka 'int') [-Wint-to-void-pointer-cast]
TagsNo tags attached.
Test case numberbugs fclasses(002) bug24863_1, bug24863_2, bug24863_3

Activities

ski

2014-05-20 14:57

developer   ~0029411

Warning was fixed.
Changes are located in branch CR24863.
Please, review.

abv

2014-05-20 17:38

manager   ~0029416

I suggest more in-depth change:
- remove class VMap in Draw (it is data map of int to Drawable3D defined in CDL)
- in Draw_VariableCommands.cxx, use NCollection_DataMap<TCollection_AsciiString,Handle(Draw_Drawable3D)> instead of Draw_VMap
- use name instead of ClientData to associate Tcl variable with the object

Then two related issues can be fixed:

1. When DRAW variable is deleted, corresponding Drawable object should be deleted. Now it is not the case, as the map theVariables is never cleared.

Current implementation thus shows memory leak visible when the same variable is set many times, for instance, creating a box 100.000 times leaks ~ 5 Mb:

Draw[1]> pload MODELING
Draw[2]> meminfo heap
1262810
Draw[3]> for {set i 1} {$i < 100000} {incr i} {box b 10 10 10}
Draw[4]> meminfo heap
6076490

Most likely the right place to remove the object is in tracevar().

2. DRAW variable can be changed to different value using Tcl set command, after that some DRAW functionality becomes broken. For instance:

Draw[5]> av; fit
Draw[6]> whatis .
Pick an object
b is a shape SOLID FORWARD Free Modified

Draw[7]> set b aaaaa
aaaaa
Draw[8]> whatis .
Pick an object
9 is a

I suppose change of the DRAW variable should also be traced, to protect against change (see documentation of Tcl_TraceVar, TCL_TRACE_WRITES).

3. At the end, I suppose the code related to support of versions of Tcl less than 8.4 should be cleared

abv

2014-05-22 09:22

manager   ~0029454

Besides, test bugs caf bug23489 does very similar thing: loads the same shape in cycle and measures memory change. From this test it seems that at least shape IS deallocated. It would be nice to understand how it works (perhaps in debugger), and see how the fix will affect that test case (whether minor memory leak still observed in it disappears).

ski

2014-05-22 18:53

developer   ~0029479

Remarks were applied:
    
- class VMap in Draw was removed
- NCollection_DataMap is used to store objects
- name of object is used to associate Tcl variable with the object
- creation and changing of objects are correctly handled
- minor memory leaks disappears in both test cases
- "whatis" command works correctly

Please, review.

kgv

2014-05-23 09:51

developer   ~0029480 

-static Draw_VMap theVariables;
+static NCollection_DataMap<TCollection_AsciiString,Handle(Draw_Drawable3D)> theVariables;
...
+    if (theVariables.IsBound(name))
+      D = Handle(Draw_Drawable3D)::DownCast(theVariables(name));

DownCast looks redundant here. Please use theVariables.Find (name, D) here instead.

+  Handle(Draw_Drawable3D)& D = *((Handle(Draw_Drawable3D)*)&theVariables(name));

Cast now is redundant here as well.

ski

2014-05-23 11:04

developer   ~0029484

Redundant casts were removed.
Please, review.

abv

2014-06-05 20:34

manager   ~0029720

In general the change looks Ok, and the fact that memory leak disappeared can be seen by reducing memory delta between iterations in several test cases checking memory leak to (e.g. bugs caf bug23489, bugs fclasses 7287_2..6) to zero.

However, some problems remain.

1. Protected variable still can be changed on Tcl level (I guess it is necessary to restore initial value in tracevar if variable is protected):

Draw[23]> box b 10 10 10
Draw[24]> protect b
b
Draw[25]> set b aaa
can't set "b": variable is protected
Draw[26]> puts $b
aaa
Draw[27]> whatis b
b is a shape SOLID FORWARD Free Modified

Draw[28]> av; fit
Draw[29]> whatis .
Pick an object
whatis .
 is a

2. Tests bugs fclasses bug7287_* are FAILED if executed interactively, the reason is not clear. Note that this problem is not related to the current issue, thus separate issue can be created for that...

abv

2014-06-05 20:35

manager   ~0029721

Then, please add a couple of test cases for this issue, basing on examples above.

ski

2014-06-17 16:18

developer   ~0029799

Now initial value is restored in tracevar if variable is protected.
Also, some tests were added.
Changes are located in branch CR24863.
Please, review.

kgv

2014-06-25 19:36

developer   ~0029883

Dear bugmaster,

please test patch in branch CR24863 for regressions.

mkv

2014-06-27 14:14

tester   ~0029901

Dear BugMaster,

Branch CR24863 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 7562f8c73efa09dfa3d3a922aee61741273260b2

Number of compiler warnings:

occt component :
Linux: 16 (16 on master)
Windows: 0 (0 on master)
MacOS: 199 (200 on master)

products component :
Linux: 11 (11 on master)
Windows: 2 (2 on master)

Regressions/Differences:
http://occt-tests/CR24863-master-occt/Debian60-64/summary.html
http://occt-tests/CR24863-master-occt/Windows-32-VC9/summary.html

Testing cases:
http://occt-tests/CR24863-master-occt/Windows-32-VC9/bugs/fclasses/bug24863_1.html
bugs fclasses(002) bug24863_1: OK
http://occt-tests/CR24863-master-occt/Windows-32-VC9/bugs/fclasses/bug24863_2.html
bugs fclasses(002) bug24863_2: OK
http://occt-tests/CR24863-master-occt/Windows-32-VC9/bugs/fclasses/bug24863_3.html
bugs fclasses(002) bug24863_3: OK

git

2014-07-16 13:35

administrator   ~0030186

Branch CR24863 has been updated forcibly by ski.

SHA-1: 5e1600c77474b47a60018db6ba42432a626e88c0

ski

2014-07-16 13:43

developer   ~0030187

Regressions were fixed.

Some remarks:
    test case offset faces_type_i C9 - it seems that it is just instability.

    test cases bugs modalg_5 bug24157_8 and bug24157_9 (perfomance tests) should work properly.
    have a look at test cases:
        check_time for bug24157_8 is equal to 10 seconds and execution time
        of command bbuild is equal to 8.89 (from test report):
            bbuild result -t
            Tps: 8.89

        check_time for bug24157_9 is equal to 100 seconds and execution time
        of command bbuild is equal to 72.58 (from test report):
            bbuild result -t
            Tps: 72.58
    In both cases execution time is less than check_time.
    So, tests should be OK.

    test case bugs vis bug79 is OK on current master with my changes on my station:
        vdisplay s
        meminfo h
        81068012
        verase s
        vdisplay s
        meminfo h
        81068012
        verase s
        vdisplay s
        meminfo h
        81068012
        Checking trend: nb = 3, mean delta = 0.0, sigma = 0.0
        No memory leak, 3 iterations

Changes are located in branch CR24863.
please, review.

git

2014-07-22 15:54

administrator   ~0030308

Branch CR24863 has been deleted by inv.

SHA-1: 5e1600c77474b47a60018db6ba42432a626e88c0

git

2014-08-05 19:02

administrator   ~0030585

Branch CR24863_restored has been created by abv.

SHA-1: 5e1600c77474b47a60018db6ba42432a626e88c0


This branch includes the following new commits:

       new 17aa9fb 0024863: CLang warnings -Wint-to-void-pointer-cast
       new ff2b19e Remarks were applied.
       new 0089fc9 Redundant casts were removed.
       new 643eac0 Initial value is restored if variable is protected. Tests for bug 0024863 were added.
       new 5e1600c Some test cases and tcl command "save" were improved.


Detailed log of new commits:

commit 5e1600c77474b47a60018db6ba42432a626e88c0
Author: ski
Date: Wed Jul 16 13:34:41 2014 +0400

    Some test cases and tcl command "save" were improved.

commit 643eac01e97f156d94f4b9edced307ffeaa03230
Author: ski
Date: Tue Jun 17 15:12:08 2014 +0400

    Initial value is restored if variable is protected.
    Tests for bug 0024863 were added.

commit 0089fc9cf305d140c45f47b9a03c504a7752ece5
Author: ski
Date: Fri May 23 11:03:09 2014 +0400

    Redundant casts were removed.

commit ff2b19ec81dbab1a9567bcc5e47fbe592995ef76
Author: ski
Date: Thu May 22 17:48:28 2014 +0400

    Remarks were applied.
    
    - class VMap in Draw was removed
    - NCollection_DataMap is used to store objects
    - name of object is used to associate Tcl variable with the object
    - creation and changing of objects are correclty handled

commit 17aa9fb0153ca7a5360cd33abc0b3b364c9438b8
Author: ski
Date: Tue May 20 14:52:55 2014 +0400

    0024863: CLang warnings -Wint-to-void-pointer-cast
    
    Warning was fixed.

abv

2014-08-05 19:16

manager   ~0030588

Reviewed, please test.

Actually I have some doubts whether changes made in the last commit in test scripts (except in tests/bugs/*) are optimal or even correct; the use of upvar seems to me too tricky. Would not it be easier to avoid upvar and just use directly the argument?

git

2014-08-06 11:36

administrator   ~0030598

Branch CR24863_restored has been updated forcibly by apv.

SHA-1: 1bd7873f8a29329c002e60505760e4381963be79

apv

2014-08-07 12:59

tester   ~0030617

Branch CR24863 has been created for OCCTProducts component

SHA-1: b4addf232bc41db1354c72ad866b0e9116e36021
   0024863: CLang warnings -Wint-to-void-pointer-cast

   Update for OCCTProducts component

git

2014-08-07 13:08

administrator   ~0030618

Branch CR24863_restored has been updated by ski.

SHA-1: 45ce7a51445e61bf5d1bf4312d1d2d8ad4307bad


      from 1bd7873 Some test cases and tcl command "save" were improved.
       new 45ce7a5 Useless using of upvar was removed.


Detailed log of new commits:

commit 45ce7a51445e61bf5d1bf4312d1d2d8ad4307bad
Author: ski
Date: Thu Aug 7 13:07:59 2014 +0400

    Useless using of upvar was removed.

apv

2014-08-07 13:14

tester   ~0030619

Last edited: 2014-08-07 13:24

 Dear BugMaster,

Branch CR24863_restored (and products from CR24863) was compiled on Linux and Windows platforms and tested.
SHA-1: 1bd7873f8a29329c002e60505760e4381963be79
SHA-1: b4addf232bc41db1354c72ad866b0e9116e36021

Number of compiler warnings:
occt component
   Linux: 15 (15 on master)
   Windows: 0 (0 on master)
products component :
   Linux: 11 (11 on master)
   Windows: 1 (1 on master)

Regressions/Differences:
Not detected

Testing cases:
bugs fclasses bug24863_1 - OK
http://occt-tests/CR24863-restored-master-occt/Windows-32-VC10/bugs/fclasses/bug24863_1.html
bugs fclasses bug24863_2 - OK
http://occt-tests/CR24863-restored-master-occt/Windows-32-VC10/bugs/fclasses/bug24863_2.html
bugs fclasses bug24863_3 - OK
http://occt-tests/CR24863-restored-master-occt/Windows-32-VC10/bugs/fclasses/bug24863_3.html

Testing on Linux:
Total MEMORY difference: 352030184 / 351944952
Total CPU difference: 41376.18 / 46365.40000000032

Testing on Windows:
Total MEMORY difference: 238936208 / 239990088
Total CPU difference: 31207.09375 / 30887.765625

There are differences in images found by testdiff:
http://occt-tests/CR24863-restored-master-occt/Debian60-64/diff-Debian60-64.html
http://occt-tests/CR24863-restored-master-occt/Windows-32-VC10/diff-Windows-32-VC10.html

git

2014-08-18 11:07

administrator   ~0030816

Branch CR24863_restored has been deleted by inv.

SHA-1: 45ce7a51445e61bf5d1bf4312d1d2d8ad4307bad

Related Changesets

occt: master 1951a27c

2014-08-07 10:31:10

ski


Committer: apv Details Diff		 0024863: CLang warnings -Wint-to-void-pointer-cast

Warning was fixed.

Remarks were applied.

- class VMap in Draw was removed
- NCollection_DataMap is used to store objects
- name of object is used to associate Tcl variable with the object
- creation and changing of objects are correclty handled

Redundant casts were removed.

Initial value is restored if variable is protected.
Tests for bug 0024863 were added.

Some test cases and tcl command "save" were improved.

Useless using of upvar was removed.		 Affected Issues
0024863
mod - src/Draw/Draw.cdl Diff File
mod - src/Draw/Draw_VariableCommands.cxx Diff File
mod - src/DrawResources/StandardCommands.tcl Diff File
add - tests/bugs/fclasses/bug24863_1 Diff File
add - tests/bugs/fclasses/bug24863_2 Diff File
add - tests/bugs/fclasses/bug24863_3 Diff File
mod - tests/bugs/modalg_2/bug298 Diff File
mod - tests/bugs/modalg_2/bug410_4 Diff File
mod - tests/bugs/modalg_2/bug485 Diff File
mod - tests/bugs/modalg_2/bug487 Diff File
mod - tests/bugs/modalg_2/bug5805_1 Diff File
mod - tests/bugs/moddata_2/bug23165 Diff File
mod - tests/bugs/moddata_2/bug332 Diff File
mod - tests/bugs/xde/bug945 Diff File
mod - tests/caf/nam/A5 Diff File
mod - tests/geometry/begin Diff File
mod - tests/mesh/end Diff File
mod - tests/offset/faces_type_i/C9 Diff File
mod - tests/xml/begin Diff File

Issue History

Date Modified Username Field Change
2014-04-21 15:30 ski New Issue
2014-04-21 15:30 ski Assigned To => ski
2014-04-21 15:30 ski Status new => assigned
2014-05-20 14:57 ski Note Added: 0029411
2014-05-20 14:57 ski Assigned To ski => abv
2014-05-20 14:57 ski Status assigned => resolved
2014-05-20 17:38 abv Note Added: 0029416
2014-05-20 17:38 abv Assigned To abv => ski
2014-05-20 17:38 abv Status resolved => assigned
2014-05-20 17:43 abv Target Version => 6.8.0
2014-05-22 09:22 abv Note Added: 0029454
2014-05-22 18:53 ski Note Added: 0029479
2014-05-22 18:53 ski Assigned To ski => abv
2014-05-22 18:53 ski Status assigned => resolved
2014-05-23 09:51 kgv Note Added: 0029480
2014-05-23 10:20 kgv Assigned To abv => ski
2014-05-23 10:20 kgv Status resolved => assigned
2014-05-23 11:04 ski Note Added: 0029484
2014-05-23 11:04 ski Assigned To ski => abv
2014-05-23 11:04 ski Status assigned => resolved
2014-06-05 20:34 abv Note Added: 0029720
2014-06-05 20:34 abv Assigned To abv => ski
2014-06-05 20:34 abv Status resolved => assigned
2014-06-05 20:35 abv Note Added: 0029721
2014-06-17 16:18 ski Note Added: 0029799
2014-06-17 16:18 ski Assigned To ski => abv
2014-06-17 16:18 ski Status assigned => resolved
2014-06-20 11:23 ski Assigned To abv => kgv
2014-06-25 19:36 kgv Note Added: 0029883
2014-06-25 19:36 kgv Assigned To kgv => bugmaster
2014-06-25 19:36 kgv Status resolved => feedback
2014-06-26 12:35 bugmaster Assigned To bugmaster => mkv
2014-06-26 12:35 bugmaster Status feedback => reviewed
2014-06-27 14:14 mkv Note Added: 0029901
2014-06-27 14:15 mkv Test case number => bugs fclasses(002) bug24863_1, bug24863_2, bug24863_3
2014-06-27 14:15 mkv Assigned To mkv => ski
2014-06-27 14:15 mkv Status reviewed => assigned
2014-07-16 13:35 git Note Added: 0030186
2014-07-16 13:43 ski Note Added: 0030187
2014-07-16 13:43 ski Assigned To ski => abv
2014-07-16 13:43 ski Status assigned => resolved
2014-07-22 15:54 git Note Added: 0030308
2014-08-05 19:02 git Note Added: 0030585
2014-08-05 19:16 abv Note Added: 0030588
2014-08-05 19:16 abv Assigned To abv => bugmaster
2014-08-05 19:16 abv Status resolved => reviewed
2014-08-06 11:36 git Note Added: 0030598
2014-08-07 12:59 apv Note Added: 0030617
2014-08-07 13:08 git Note Added: 0030618
2014-08-07 13:14 apv Note Added: 0030619
2014-08-07 13:18 apv Note Edited: 0030619
2014-08-07 13:24 apv Note Edited: 0030619
2014-08-07 13:27 apv Status reviewed => tested
2014-08-08 13:51 apv Changeset attached => occt master 1951a27c
2014-08-08 13:51 apv Assigned To bugmaster => apv
2014-08-08 13:51 apv Status tested => verified
2014-08-08 13:51 apv Resolution open => fixed
2014-08-18 11:07 git Note Added: 0030816
2014-11-11 12:45 aiv Fixed in Version => 6.8.0
2014-11-11 12:57 aiv Status verified => closed