Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review/all warning fixes #41

Merged
merged 27 commits into from
Jun 14, 2011
Merged

Review/all warning fixes #41

merged 27 commits into from
Jun 14, 2011

Conversation

QbProg
Copy link
Collaborator

@QbProg QbProg commented Jun 13, 2011

This includes:

  • Changes applyed to previous pull request by Denis
  • "Interval" behavoir fix (pourSGI)
  • Some other warning fixes done yesterday, mainly uninitialized vars

I've reviewed again the changes, and there should not be evident problems nor breaking changes.
Someone can just double-check them?

tpaviot and others added 27 commits June 11, 2011 14:10
Removed unused-local-var identified with XCode/gcc 4.2.1
Removed lines do not contain method calls (or call methods
which are known to have no side effect, like Abs), these
changes should thus be safe.

Fixes deal with all Toolkits as well as DRAW.
Removed PArrayPrint and PArrayInquire declared static
but never defined in OpenGl_PrimitiveArray.cxx
Removed unused functions raising defined but not used warning
Fixed warning Label 'ERR' defined but not used
Fixed format %d expects type int but argument XX is long unsigned int
Fix warning 'myVar' *may* be used uninitialized in this function
Fix warning 'myVar' *is* used uninitialized in this function
Removed unused-local-var identified with XCode/gcc 4.2.1.
Lines removed call methods which may have side effects, so this
commit must be double-checked.

Fixes deal with all Toolkits as well as DRAW.

(QbProg : included only commits without side effects )
Fixed comparison between signed and unsigned int warnings

(QbProg: changed some modifications to be done in 0.5)
Fixed warning: deprecated conversion from string constant to 'char*'
Removed unuinitialized vars from TKIGES
Removed old-style C declarators from TKIges
Fixes (and todos) in TKStepBase
Fixes many potential uninitialized vars around Draw and Test
Fixed some assignments in conditional expressions
Fixed some unreachable code warnings
This includes a buffer overflow in Interface_MSG
Removed unused function (WNT)
Fixed many potentially un-init vars
Removed unreachable code warnings, and marked with @todo when in doubt
Small one for now. In target-0.5 the function should return char instead of Standard_Integer
Also fixed a bug in TrimShellCorner due to a missing ==
@tpaviot
Copy link
Owner

tpaviot commented Jun 14, 2011

+1

@dbarbier
Copy link
Collaborator

+1

My only concern is that there are 4 commits modifying IntCurve_IntConicConic_Tool.cxx, but

$ git diff master origin/review/all-warning-fixes -- src/IntCurve/IntCurve_IntConicConic_Tool.cxx
diff --git a/src/IntCurve/IntCurve_IntConicConic_Tool.cxx b/src/IntCurve/IntCurve_IntConicConic_Tool.cxx
index eeefd34..dc2c904 100644
--- a/src/IntCurve/IntCurve_IntConicConic_Tool.cxx
+++ b/src/IntCurve/IntCurve_IntConicConic_Tool.cxx
@@ -138,7 +138,8 @@ PeriodicInterval PeriodicInterval::SecondIntersection(PeriodicInterval& PInter)
   return(PeriodicInterval(a,b));
 }
 //----------------------------------------------------------------------
-Interval::Interval() { IsNull=Standard_True; }
+Interval::Interval() : Binf(0.0), Bsup(0.0),HasFirstBound(Standard_False), HasLastBound(Standard_False)
+{ IsNull=Standard_True; }

 Interval::Interval(const Standard_Real a,const Standard_Real b) {
   HasFirstBound=HasLastBound=Standard_True;

So it would be better to rewrite this branch again to avoid do/undo commits, but if you do not know how to do that, I won't oppose this merge, it is better to have this branch merged as soon as possible.

@QbProg
Copy link
Collaborator Author

QbProg commented Jun 14, 2011

Sorry, that's my fault, the correction took 3 commits instead of 1 due
to stupid errors.

I imagine how to rewrite it (rebase -i I guess :), but will not have
the time to do this really soon.
So if someone else has the chance to do this in short time, we'll
wait, else for this time let's just merge the branch and look forward
for other
changes.

@dbarbier
Copy link
Collaborator

As I wrote in my previous comment, no problem for me, and when you have time, take a few minutes to learn to use git rebase -i, no doubt you will find this command incredibly awesome. But be warned that you may then want to rewrite everybody's branches to improve them, this is addictive ;-)

QbProg added a commit that referenced this pull request Jun 14, 2011
@QbProg QbProg merged commit 5e184f9 into master Jun 14, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants