View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0022777 | Open CASCADE | OCCT:Visualization | public | 2011-10-24 16:27 | 2019-09-15 10:55 |
Reporter | Assigned To | bugmaster | |||
Priority | low | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Platform | A | OS | L | ||
Product Version | 6.5.2 | ||||
Target Version | 7.4.0 | Fixed in Version | 7.4.0 | ||
Summary | 0022777: Visualization - Unsafe way to get attribute values from MeshVS_Drawer | ||||
Description | There are numerous locations in MeshVS package code where some attribute's value is obtained with help of MeshVS_Drawer::Get****() methods but the return status of these methods is not checked. Meanwhile, the local variables that are used to store the attribute values are often not initialized properly. This is done in assumption that the drawer instance has been initialized properly, but the drawer class itself does not have this initialization logic. This leads to a situation when many local variables might potentially hold undefined values. For (hopefully) all supported drawer attributes, MeshVS_Mesh constructor sets some meaningful defaults. However, if MeshVS_Mesh::SetDrawer() is called by an application and the new drawer is not initialized with good defaults for all supported attributes somewhere outside MeshVS package, this might lead to the above-mentioned troubles. To eliminate completely this kind of problems, it is necessary to review MeshVS package sources and provide some initializers for the local variables used to store values obtained from the drawer. Severity Code Description Project File Line Suppression State Warning C6031 Return value ignored: 'MeshVS_Drawer::GetColor'. TKMeshVS occt_vc14\src\meshvs\meshvs_vectorprsbuilder.cxx 326 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetInteger'. TKMeshVS occt_vc14\src\meshvs\meshvs_commonsensitiveentity.cxx 37 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetColor'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 190 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetColor'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 191 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetColor'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 192 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetDouble'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 193 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetDouble'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 194 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 195 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 196 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 197 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_elementalcolorprsbuilder.cxx 198 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_mesh.cxx 151 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_mesh.cxx 251 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 117 Warning C6255 _alloca indicates failure by raising a stack overflow exception. Consider using _malloca instead. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 250 Warning C6255 _alloca indicates failure by raising a stack overflow exception. Consider using _malloca instead. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 251 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetDouble'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 199 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 208 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 209 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 211 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetMaterial'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 219 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 246 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetMaterial'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 535 Warning C6255 _alloca indicates failure by raising a stack overflow exception. Consider using _malloca instead. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 857 Warning C6263 Using _alloca in a loop: this can quickly overflow stack.: Lines: 852 TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 857 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_meshprsbuilder.cxx 1062 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_nodalcolorprsbuilder.cxx 164 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_nodalcolorprsbuilder.cxx 165 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetColor'. TKMeshVS occt_vc14\src\meshvs\meshvs_nodalcolorprsbuilder.cxx 428 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetColor'. TKMeshVS occt_vc14\src\meshvs\meshvs_nodalcolorprsbuilder.cxx 429 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetColor'. TKMeshVS occt_vc14\src\meshvs\meshvs_nodalcolorprsbuilder.cxx 430 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetDouble'. TKMeshVS occt_vc14\src\meshvs\meshvs_nodalcolorprsbuilder.cxx 431 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_nodalcolorprsbuilder.cxx 432 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetBoolean'. TKMeshVS occt_vc14\src\meshvs\meshvs_nodalcolorprsbuilder.cxx 479 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetColor'. TKMeshVS occt_vc14\src\meshvs\meshvs_textprsbuilder.cxx 183 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetDouble'. TKMeshVS occt_vc14\src\meshvs\meshvs_textprsbuilder.cxx 184 Warning C6031 Return value ignored: 'MeshVS_Drawer::GetDouble'. TKMeshVS occt_vc14\src\meshvs\meshvs_textprsbuilder.cxx 185 | ||||
Steps To Reproduce | N/A | ||||
Tags | No tags attached. | ||||
Test case number | Not required | ||||
|
Dear kgv, could you please put your opinion? |
|
Branch CR22777 has been created by kgv. SHA-1: acb3cdf11b8cda1548a128001dc20cecb133418d Detailed log of new commits: Author: kgv Date: Fri Jul 21 10:51:06 2017 +0300 0022777: Visualization - Unsafe way to get attribute values from MeshVS_Drawer Define auxiliary macros Standard_CHECK_RETURN for asking compiler to warn about unused result. |
|
Branch CR22777_1 has been created by mzernova. SHA-1: e33f2123b022a6215a4a7c56cfd74eebfbd3826e Detailed log of new commits: Author: mzernova Date: Fri Sep 6 13:56:40 2019 +0300 0022777: Visualization - Unsafe way to get attribute values from MeshVS_Drawer Were provide some initializers for the local variables used to store values obtained from the drawer. |
|
The patch CR22777 is ready to review |
|
- Standard_Real anEdgeWidth, aLineWidth; + Standard_Real anEdgeWidth = 1.0; + Standard_Real aLineWidth = 1.0; It is not necessary splitting declarations for primitive types. Keeping single line is acceptable and sometimes preferable for long lists of valeus. > Standard_Real anEdgeWidth = 1.0, aLineWidth = 1.0; |
|
Standard_Integer anEdgeInt, aLineInt; ... if ( aDrawer->GetInteger ( MeshVS_DA_EdgeType, anEdgeInt) ) anEdgeType = (Aspect_TypeOfLine) anEdgeInt; if ( aDrawer->GetInteger ( MeshVS_DA_BeamType, aLineInt) ) aLineType = (Aspect_TypeOfLine) aLineInt; Although this is not a bug, it is preferable initializing all values, in case if it is not time-critical code. And these temporary variables are better scoped close to the place where they are used. { Standard_Integer anEdgeInt = Aspect_TOL_SOLID; if (aDrawer->GetInteger (MeshVS_DA_EdgeType, anEdgeInt)) { anEdgeType = (Aspect_TypeOfLine )anEdgeInt; } } |
|
+ IsReflect, IsMeshSmoothShading = Standard_False; If variable initialization happens afterwards, it is better moving declaration as well. > const Standard_Boolean IsReflect = (IsMeshReflect && !HasHilightFlag); |
|
> Were provide some initializers for the local variables used to store values obtained from the drawer. Please revise message in git commit, I'm unable to understand it. |
|
Branch CR22777_1 has been updated by mzernova. SHA-1: 101f020db310f52bf3cb6ed6fee8380ed3853f6d Detailed log of new commits: Author: mzernova Date: Mon Sep 9 12:57:09 2019 +0300 Some default values were set for local variables used for storing values that are obtained by MeshVS_Drawer::Get****() methods remarks from kgv |
|
The patch CR22777 is ready to review |
|
Branch CR22777_2 has been created by osa. SHA-1: 7d20640e957d4a70b9768a7f7cada5d74828c417 Detailed log of new commits: Author: mzernova Date: Fri Sep 6 13:56:40 2019 +0300 0022777: Visualization - Unsafe way to get attribute values from MeshVS_Drawer Set default values for local variables used for storing values that are obtained by MeshVS_Drawer::Get****() methods |
|
http://jenkins-test-12.nnov.opencascade.com:8080/view/CR22777_2-master-OSA |
|
Combination - OCCT branch : CR22777_2 master SHA - 7d20640e957d4a70b9768a7f7cada5d74828c417 5f5b1aed1c6e139bbd34314eca77ae7abcd8895c Products branch : master SHA - f9d0bd5e3a29d6a97b3f5f6354ea2397253ab4f8 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: Debian80-64: OCCT Total CPU difference: 16840.780000000097 / 16836.290000000165 [+0.03%] Products Total CPU difference: 10560.550000000048 / 10553.510000000031 [+0.07%] Windows-64-VC14: OCCT Total CPU difference: 18310.5625 / 18300.0625 [+0.06%] Products Total CPU difference: 12356.90625 / 12227.453125 [+1.06%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
Branch CR22777_2 has been deleted by inv. SHA-1: 7d20640e957d4a70b9768a7f7cada5d74828c417 |
|
Branch CR22777_1 has been deleted by inv. SHA-1: 101f020db310f52bf3cb6ed6fee8380ed3853f6d |
|
Branch CR22777 has been deleted by inv. SHA-1: acb3cdf11b8cda1548a128001dc20cecb133418d |
occt: master b1492cb3 2019-09-06 10:56:40
Committer: bugmaster Details Diff |
0022777: Visualization - Unsafe way to get attribute values from MeshVS_Drawer Set default values for local variables used for storing values that are obtained by MeshVS_Drawer::Get****() methods |
Affected Issues 0022777 |
|
mod - src/MeshVS/MeshVS_CommonSensitiveEntity.cxx | Diff File | ||
mod - src/MeshVS/MeshVS_ElementalColorPrsBuilder.cxx | Diff File | ||
mod - src/MeshVS/MeshVS_MeshPrsBuilder.cxx | Diff File | ||
mod - src/MeshVS/MeshVS_NodalColorPrsBuilder.cxx | Diff File | ||
mod - src/MeshVS/MeshVS_TextPrsBuilder.cxx | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2011-10-24 16:27 |
|
New Issue | |
2011-10-24 16:27 |
|
Assigned To | => san |
2017-07-12 18:46 |
|
Note Added: 0068226 | |
2017-07-12 18:46 |
|
Assigned To | san => kgv |
2017-07-12 18:46 |
|
Status | new => assigned |
2017-07-21 10:49 | kgv | Description Updated | |
2017-07-21 10:49 | kgv | Steps to Reproduce Updated | |
2017-07-21 10:50 | kgv | Summary | Unsafe way to get attribute values from MeshVS_Drawer => Visualization - Unsafe way to get attribute values from MeshVS_Drawer |
2017-07-21 10:53 | git | Note Added: 0068532 | |
2017-07-21 10:54 | kgv | Target Version | => 7.3.0 |
2018-03-13 15:04 | kgv | Target Version | 7.3.0 => 7.4.0 |
2019-08-13 12:04 | kgv | Target Version | 7.4.0 => 7.5.0 |
2019-08-27 14:39 | kgv | Relationship added | related to 0030448 |
2019-08-27 19:46 | kgv | Assigned To | kgv => user897 |
2019-09-06 14:21 | git | Note Added: 0086875 | |
2019-09-06 14:28 |
|
Note Added: 0086876 | |
2019-09-06 14:28 |
|
Assigned To | user897 => osa |
2019-09-06 14:28 |
|
Status | assigned => resolved |
2019-09-06 14:32 | kgv | Note Added: 0086878 | |
2019-09-06 14:36 | kgv | Note Added: 0086881 | |
2019-09-06 14:39 | kgv | Note Added: 0086883 | |
2019-09-06 14:48 | kgv | Note Added: 0086885 | |
2019-09-06 14:48 | kgv | Assigned To | osa => user897 |
2019-09-06 14:48 | kgv | Status | resolved => assigned |
2019-09-06 14:48 | kgv | Target Version | 7.5.0 => 7.4.0 |
2019-09-10 17:41 | git | Note Added: 0086995 | |
2019-09-10 17:42 |
|
Note Added: 0086996 | |
2019-09-10 17:42 |
|
Assigned To | user897 => osa |
2019-09-10 17:42 |
|
Status | assigned => resolved |
2019-09-13 12:15 | git | Note Added: 0087044 | |
2019-09-13 15:07 |
|
Note Added: 0087061 | |
2019-09-13 15:08 |
|
Assigned To | osa => bugmaster |
2019-09-13 15:08 |
|
Status | resolved => reviewed |
2019-09-13 20:19 | bugmaster | Note Added: 0087074 | |
2019-09-13 20:19 | bugmaster | Status | reviewed => tested |
2019-09-13 20:19 | bugmaster | Test case number | => Not required |
2019-09-15 10:51 | bugmaster | Changeset attached | => occt master b1492cb3 |
2019-09-15 10:51 | bugmaster | Status | tested => verified |
2019-09-15 10:51 | bugmaster | Resolution | open => fixed |
2019-09-15 10:55 | git | Note Added: 0087093 | |
2019-09-15 10:55 | git | Note Added: 0087099 | |
2019-09-15 10:55 | git | Note Added: 0087105 |