View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0022885CommunityOCCT:Foundation Classespublic2012-01-03 23:282012-03-29 17:26
Reporterbarbier Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformAOSL 
Product Version6.5.2 
Target Version6.5.3Fixed in Version6.5.3 
Summary0022885: Bugfix: else clause applies to the wrong if statement because of missing braces
DescriptionHello,

Source code contains several occurences of this pattern:

  if (condition1)
    if (condition2)
      do_something
  else

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

  if (condition1) {
    if (condition2)
      do_something
 } else

See proposed changes in bugfix-misplaced-else.patch
Additional information
and documentation updates		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.
TagsNo tags attached.
Test case numberTest case is not required

Attached Files

  • bugfix-misplaced-else.patch (4,664 bytes)

Activities

barbier

2012-01-03 23:28

developer  

bugfix-misplaced-else.patch (4,664 bytes)

abv

2012-01-10 11:10

manager   ~0019065

The fix is integrated to branch OCC22885, ready for testing

Sergey, could you please review the corrections made in AIS* and TPrsStd packages?

san

2012-01-11 21:11

developer   ~0019092

The issue reviewed with the following remarks:

src/AIS2D/AIS2D_InteractiveContext.cxx:
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.

src/AIS/AIS_InteractiveContext.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!

san

2012-01-11 21:14

developer   ~0019093

SVN branch OCC22885 has been updated and is ready for testing.

barbier

2012-01-13 20:37

developer   ~0019125

gcc 4.6 (maybe earlier versions too?) prints a warning when source code contains:

  if (foo)
    if (bar)
    else

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.

barbier

2012-01-13 20:39

developer   ~0019126

BTW since you removed AIS2D_InteractiveContext::EraseMode, maybe you could also remove AIS_InteractiveContext::EraseMode? It seems to be unused too.

apn

2012-01-17 10:52

administrator   ~0019143

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

ama

2012-02-08 17:08

developer   ~0019466

SVN branch http://svn/svn/occt/branches/OCC22885 (revision 10369) is ready to be reviewed.

abv

2012-02-09 13:11

manager   ~0019487

No remarks, please test

apn

2012-02-15 15:36

administrator   ~0019578

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

bugmaster

2012-02-17 15:20

administrator   ~0019627

Integrated into trunk of occt repository

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

Modified:
   trunk/src/AIS/AIS_InteractiveContext.cxx
   trunk/src/AIS2D/AIS2D_InteractiveContext.cdl
   trunk/src/AIS2D/AIS2D_InteractiveContext.cxx
   trunk/src/Bnd/Bnd_Box.cxx
   trunk/src/Bnd/Bnd_Box2d.cxx
   trunk/src/IGESData/IGESData_GlobalSection.cxx
   trunk/src/ShapeAnalysis/ShapeAnalysis_CheckSmallFace.cxx
   trunk/src/TPrsStd/TPrsStd_AISPresentation.cxx

Related Changesets

occt: master 44d9ae89

2012-02-17 11:18:31

ama


Committer: bugmaster Details Diff		 0022885: Bugfix: else clause applies to the wrong if statement because of missing braces Affected Issues
0022885
mod - src/AIS/AIS_InteractiveContext.cxx Diff File
mod - src/AIS2D/AIS2D_InteractiveContext.cdl Diff File
mod - src/AIS2D/AIS2D_InteractiveContext.cxx Diff File
mod - src/Bnd/Bnd_Box.cxx Diff File
mod - src/Bnd/Bnd_Box2d.cxx Diff File
mod - src/IGESData/IGESData_GlobalSection.cxx Diff File
mod - src/ShapeAnalysis/ShapeAnalysis_CheckSmallFace.cxx Diff File
mod - src/TPrsStd/TPrsStd_AISPresentation.cxx Diff File

Issue History

Date Modified Username Field Change
2012-01-03 23:28 barbier New Issue
2012-01-03 23:28 barbier Assigned To => abv
2012-01-03 23:28 barbier File Added: bugfix-misplaced-else.patch
2012-01-05 12:03 abv Status new => assigned
2012-01-06 18:58 barbier Status assigned => resolved
2012-01-10 10:59 abv Assigned To abv => san
2012-01-10 11:10 abv Note Added: 0019065
2012-01-11 21:11 san Note Added: 0019092
2012-01-11 21:14 san Note Added: 0019093
2012-01-11 21:14 san Assigned To san => bugmaster
2012-01-11 21:14 san Status resolved => reviewed
2012-01-11 21:19 san Additional Information Updated
2012-01-13 20:37 barbier Note Added: 0019125
2012-01-13 20:39 barbier Note Added: 0019126
2012-01-16 12:02 mkv Test case number => Test case is not required
2012-01-17 10:52 apn Note Added: 0019143
2012-01-17 10:54 apn Test case number Test case is not required => Test case is not required
2012-01-17 10:54 apn Assigned To bugmaster => san
2012-01-17 10:54 apn Status reviewed => assigned
2012-01-17 10:55 apn Assigned To san => abv
2012-01-19 20:45 abv Assigned To abv => ama
2012-02-08 17:08 ama Note Added: 0019466
2012-02-08 17:09 ama Assigned To ama => abv
2012-02-08 17:09 ama Status assigned => resolved
2012-02-09 13:11 abv Note Added: 0019487
2012-02-09 13:11 abv Assigned To abv => ama
2012-02-09 13:11 abv Status resolved => reviewed
2012-02-14 13:19 mkv Assigned To ama => aan
2012-02-14 13:19 mkv Assigned To aan => apn
2012-02-15 15:36 apn Note Added: 0019578
2012-02-15 15:37 apn Status reviewed => tested
2012-02-17 15:20 bugmaster Assigned To apn => barbier
2012-02-17 15:20 bugmaster Target Version => 6.5.3
2012-02-17 15:20 bugmaster Note Added: 0019627
2012-02-17 15:20 bugmaster Status tested => verified
2012-02-17 15:20 bugmaster Resolution open => fixed
2012-03-29 17:26 bugmaster Changeset attached => occt master 44d9ae89