Archived issue #0024863
CLang warnings -Wint-to-void-pointer-cast
Description
CLang 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]
Draw_VariableCommands.cxx:803:15: warning: cast to 'void *' from smaller integer type 'Standard_Integer' (aka 'int') [-Wint-to-void-pointer-cast]
Public activity
21 archived notes
Participants are labeled by their role within this record.
Warning was fixed.
Changes are located in branch CR24863.
Please, review.
Changes are located in branch CR24863.
Please, review.
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
- 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
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).
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.
- 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.
-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.
Redundant casts were removed.
Please, review.
Please, review.
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...
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...
Then, please add a couple of test cases for this issue, basing on examples above.
Now initial value is restored in tracevar if variable is protected.
Also, some tests were added.
Changes are located in branch CR24863.
Please, review.
Also, some tests were added.
Changes are located in branch CR24863.
Please, review.
Dear Commenter 3,
please test patch in branch CR24863 for regressions.
please test patch in branch CR24863 for regressions.
Dear Commenter 3,
Branch CR24863 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]
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
Branch CR24863 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]
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
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
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.
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.
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Branch [archived branch] has been created by Participant.
[revision removed]
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:
[revision removed]
Author: Author
Date: Wed Jul 16 13:34:41 2014 +0400
Some test cases and tcl command "save" were improved.
[revision removed]
Author: Author
Date: Tue Jun 17 15:12:08 2014 +0400
Initial value is restored if variable is protected.
Tests for bug #0024863 were added.
[revision removed]
Author: Author
Date: Fri May 23 11:03:09 2014 +0400
Redundant casts were removed.
[revision removed]
Author: Author
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
[revision removed]
Author: Author
Date: Tue May 20 14:52:55 2014 +0400
0024863: CLang warnings -Wint-to-void-pointer-cast
Warning was fixed.
[revision removed]
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:
[revision removed]
Author: Author
Date: Wed Jul 16 13:34:41 2014 +0400
Some test cases and tcl command "save" were improved.
[revision removed]
Author: Author
Date: Tue Jun 17 15:12:08 2014 +0400
Initial value is restored if variable is protected.
Tests for bug #0024863 were added.
[revision removed]
Author: Author
Date: Fri May 23 11:03:09 2014 +0400
Redundant casts were removed.
[revision removed]
Author: Author
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
[revision removed]
Author: Author
Date: Tue May 20 14:52:55 2014 +0400
0024863: CLang warnings -Wint-to-void-pointer-cast
Warning was fixed.
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?
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?
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
Branch [archived branch] has been created for OCCTProducts component
[revision removed]
0024863: CLang warnings -Wint-to-void-pointer-cast
Update for OCCTProducts component
[revision removed]
0024863: CLang warnings -Wint-to-void-pointer-cast
Update for OCCTProducts component
Branch [archived branch] has been updated by Author.
[revision removed]
from 1bd7873 Some test cases and tcl command "save" were improved.
new 45ce7a5 Useless using of upvar was removed.
Detailed log of new commits:
[revision removed]
Author: Author
Date: Thu Aug 7 13:07:59 2014 +0400
Useless using of upvar was removed.
[revision removed]
from 1bd7873 Some test cases and tcl command "save" were improved.
new 45ce7a5 Useless using of upvar was removed.
Detailed log of new commits:
[revision removed]
Author: Author
Date: Thu Aug 7 13:07:59 2014 +0400
Useless using of upvar was removed.
Dear Commenter 3,
Branch CR24863_restored (and products from CR24863) was compiled on Linux and Windows platforms and tested.
[revision removed]
[revision removed]
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
Branch CR24863_restored (and products from CR24863) was compiled on Linux and Windows platforms and tested.
[revision removed]
[revision removed]
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
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]