MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0022885Community[OCCT] OCCT:Foundation Classespublic2012-01-03 23:282012-03-29 17:26
Reporterbarbier 
Assigned Tobugmaster 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformAOSLOS VersionL
Product Version[OCCT] 6.5.2 
Target Version[OCCT] 6.5.3Fixed in Version[OCCT] 6.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 Filespatch file icon bugfix-misplaced-else.patch (4,664 bytes) 2012-01-03 23:28

- Relationships

-  Notes
(0019065)
abv (manager)
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?
(0019092)
san (developer)
2012-01-11 21:11

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!
(0019093)
san (developer)
2012-01-11 21:14

SVN branch OCC22885 has been updated and is ready for testing.
(0019125)
barbier (developer)
2012-01-13 20:37

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.
(0019126)
barbier (developer)
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.
(0019143)
apn (administrator)
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
(0019466)
ama (developer)
2012-02-08 17:08

SVN branch http://svn/svn/occt/branches/OCC22885 [^] (revision 10369) is ready to be reviewed.
(0019487)
abv (manager)
2012-02-09 13:11

No remarks, please test
(0019578)
apn (administrator)
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
(0019627)
bugmaster (administrator)
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

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
Timestamp: 2012-02-17 11:18:31
Author: ama
Committer: bugmaster
Details ] Diff ]
0022885: Bugfix: else clause applies to the wrong if statement because of missing braces
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 View Revisions
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


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker