MantisBT - Community
View Issue Details
0022885Community[OCCT] OCCT:Foundation Classespublic2012-01-03 23:282012-03-29 17:26
[OCCT] 6.5.2 
[OCCT] 6.5.3[OCCT] 6.5.3 
Test case is not required
0022885: Bugfix: else clause applies to the wrong if statement because of missing braces

Source code contains several occurences of this pattern:

  if (condition1)
    if (condition2)

where the else clause applies to condition1 and not condition2.
This code must be replaced by:

  if (condition1) {
    if (condition2)
 } else

See proposed changes in bugfix-misplaced-else.patch
Caught by compiling with g++ -Wall (with g++ 4.6)

API changes
AIS2D_InteractiveContext class: method EraseMode() has been removed as it was not used anywhere and did nothing.
No tags attached.
patch bugfix-misplaced-else.patch (4,664) 2012-01-03 23:28
Issue History
2012-01-03 23:28barbierNew Issue
2012-01-03 23:28barbierAssigned To => abv
2012-01-03 23:28barbierFile Added: bugfix-misplaced-else.patch
2012-01-05 12:03abvStatusnew => assigned
2012-01-06 18:58barbierStatusassigned => resolved
2012-01-10 10:59abvAssigned Toabv => san
2012-01-10 11:10abvNote Added: 0019065
2012-01-11 21:11sanNote Added: 0019092
2012-01-11 21:14sanNote Added: 0019093
2012-01-11 21:14sanAssigned Tosan => bugmaster
2012-01-11 21:14sanStatusresolved => reviewed
2012-01-11 21:19sanAdditional Information Updatedbug_revision_view_page.php?rev_id=1988#r1988
2012-01-13 20:37barbierNote Added: 0019125
2012-01-13 20:39barbierNote Added: 0019126
2012-01-16 12:02mkvTest case number => Test case is not required
2012-01-17 10:52apnNote Added: 0019143
2012-01-17 10:54apnTest case numberTest case is not required => Test case is not required
2012-01-17 10:54apnAssigned Tobugmaster => san
2012-01-17 10:54apnStatusreviewed => assigned
2012-01-17 10:55apnAssigned Tosan => abv
2012-01-19 20:45abvAssigned Toabv => ama
2012-02-08 17:08amaNote Added: 0019466
2012-02-08 17:09amaAssigned Toama => abv
2012-02-08 17:09amaStatusassigned => resolved
2012-02-09 13:11abvNote Added: 0019487
2012-02-09 13:11abvAssigned Toabv => ama
2012-02-09 13:11abvStatusresolved => reviewed
2012-02-14 13:19mkvAssigned Toama => aan
2012-02-14 13:19mkvAssigned Toaan => apn
2012-02-15 15:36apnNote Added: 0019578
2012-02-15 15:37apnStatusreviewed => tested
2012-02-17 15:20bugmasterAssigned Toapn => barbier
2012-02-17 15:20bugmasterTarget Version => 6.5.3
2012-02-17 15:20bugmasterNote Added: 0019627
2012-02-17 15:20bugmasterStatustested => verified
2012-02-17 15:20bugmasterResolutionopen => fixed
2012-03-29 17:26bugmasterChangeset attached => occt master 44d9ae89

2012-01-10 11:10   
The fix is integrated to branch OCC22885, ready for testing

Sergey, could you please review the corrections made in AIS* and TPrsStd packages?
2012-01-11 21:11   
The issue reviewed with the following remarks:

Lines 353 and 355: I do not understand why these two extra conditions are needed here. If gcc 4.6 reports a warning for misindented "if" statement at line 354, why not simply correct the indentation?
Apart from this, it seems that AIS2D_InteractiveContext::EraseMode() method does nothing and is not called from anywhere! So the optimal fix is to remove this method from both .cdl and .cxx.

Lines 2766, 2769: In fact, instead of making the two "if" cases symmetrically complicated, it is necessary to simplify them, leaving only comparison of myLastinMain and myLastinColl with anIObj. It is like this because this is done in ClearGlobal() method, so anIObj is going to go away (probably) forever, and we should not keep a single reference to it anywhere in AIS!
2012-01-11 21:14   
SVN branch OCC22885 has been updated and is ready for testing.
2012-01-13 20:37   
gcc 4.6 (maybe earlier versions too?) prints a warning when source code contains:

  if (foo)
    if (bar)

without taking whitespace into account, only the absence of curly braces is important.
This is a very useful warning because it is very error prone, as can be seen in the attached patch.
2012-01-13 20:39   
BTW since you removed AIS2D_InteractiveContext::EraseMode, maybe you could also remove AIS_InteractiveContext::EraseMode? It seems to be unused too.
2012-01-17 10:52   
Dear BugMaster,
Workbench KAS:dev:mkv-22885-occt was created from SVN branch http://svn/svn/occt/branches/OCC22885 [^] [^]
(and mkv-22885-products from trunk) and compiled on Linux platform.

There are following regressions in mkv-22885-products regarding to KAS:dev:products-20111229-opt

xigs 102 C8
xigs 102 G7
xigs 102 G8
xigs 102 G9

xigs 103 S8
xigs 103 S9
xigs 103 X4
xigs 103 Y7

xigs 104 B9

xigs 203 W9

xigs 302 C2

xigs 303 P1
xigs 303 W1

See results in /QADisk/occttests/results/KAS/dev/ mkv-22885-products_16012012/lin
See reference results in /QADisk/occttests/results/KAS/dev/products-20111229-opt_29122011/lin
See test cases in /QADisk/occttests/tests/ED
2012-02-08 17:08   
SVN branch http://svn/svn/occt/branches/OCC22885 [^] (revision 10369) is ready to be reviewed.
2012-02-09 13:11   
No remarks, please test
2012-02-15 15:36   
Dear BugMaster,
 Workbench KAS:dev:mkv-22885-occt was updated from SVN branch http://svn/svn/occt/branches/OCC22885 [^]
 (and mkv-22885-products from trunk) and compiled on Linux and Windows platforms.
 There are not regressions in mkv-22885-products regarding to KAS:dev:products-20120203-opt
 See results in /QADisk/occttests/results/KAS/dev/ mkv-22885-products_14022012/lin
 See reference results in /QADisk/occttests/results/KAS/dev/products-20120203-opt_03022012/lin
 See test cases in /QADisk/occttests/tests/ED
2012-02-17 15:20   
Integrated into trunk of occt repository

Date: 2012-02-17 15:18:31 +0400 (Fri, 17 Feb 2012)
New Revision: 10504