Difference between revisions of "2009 Winter Project Week Compiler Warnings:Slicer3 Graffiti"
Line 169: | Line 169: | ||
'''strcmp(this->GetPassword(),"") == 0)''' | '''strcmp(this->GetPassword(),"") == 0)''' | ||
{ | { | ||
+ | |- | ||
+ | | | ||
+ | warning: format ‘%g’ expects type ‘float*’, but argument 3 has type ‘double*’ | ||
+ | | | ||
+ | sscanf(line, "%*s '''%g'''", &this->ResidualFWHM); | ||
+ | | | ||
+ | sscanf(line, "%*s '''%lg'''", &this->ResidualFWHM); | ||
|- | |- | ||
|} | |} |
Revision as of 17:41, 25 December 2008
Home < 2009 Winter Project Week Compiler Warnings:Slicer3 Graffiti![]() Return to Project Week Main Page |
Key Investigators
- Bill Lorensen (His Basement)
Objective
The book, The Tipping Point, analyses trends and how they take hold. As Slicer3 grows and the number of contributors increases, there needs to be tighter controls on the quality of the system. The larger the code base, number of platforms and developers, the more need there is for attention to quality.
In the Tipping Point, a chapter on increasing crime in New York City in the 1990's describes how the trend was reversed. The city concentrated areas like cleaning graffiti from subway cars. The author claims that this and other seemingly trivial goals may have led to a reversal of the crime trend.
This project hypothesizes that compilation warnings are the graffiti of software quality. A lack of attention to this seemingly trivial goal shows a lack of attention to the overall quality of software. This project will reduce warnings to zero for a popular c++ compiler, GCC 4.3.
Approach, Plan
This project will use the DMAIC methodology of the Six Sigma management process to "Define", "Measure", "Analyze", "Improve" and "Control" compiler warnings in Slicer3. This project will concentrate on warnings produced by the GNU gcc 4.3 compiler. Similar approaches can be applied to other Slicer3 compilers.
The basic methodology (from Wikipedia) consists of the following five steps:
- Define process goals that are consistent with customer demands and the NA-MIC's strategy.
- Measure key aspects of the current process and collect relevant data.
- Analyze the data to verify cause-and-effect relationships. Determine what the relationships are, and attempt to ensure that all factors have been considered.
- Improve or optimize the process.
- Control to ensure that any deviations from target are corrected before they result in defects. Set up pilot runs to establish software quality, move on to production, set up control mechanisms and continuously monitor the process.
Progress
- Define
The goal is to eliminate all compiler warnings produced by the GCC 4.3 c++ compiler and to implement measures to prevent warnings from entering the software.
- Measure
On December 1, 2008, a clean Slicer3 build using the gcc 4.3 compiler produced 523 warnings in 173 source files.
- 278: shadowed variables
- 131: unused variable
- 29: suggest parentheses around && within ||
- 26: comparison between signed and unsigned integer expressions
- 17: deprecated conversion from string constant to ‘char*’
- 14: defined but not used
- 13: cast from type ‘const char*’ to type ‘char*’ casts away constness
- 7: format expects type t argument has type ‘double’
- 2: zero-length printf format string
- 278: shadowed variables
- Analyze
A description of each warning, why it is important and how to eliminate it will be provided.
- Improve
All Slicer3 source files with GCC warning defects will be repaired.
- Control
The Slicer3 dashboard will report the warnings each day. Individuals will receive e-mails when they introduce warnings. Other more drastic measures will be investigated. For example, the GCC compiler flag -Werror will treat warnings as errors, thus blocking compilation of any files that have defects.
Additional Data
Each shadowed variable warning produces 3 lines:
Modules/Meshing/BuildingBlock/vtkKWMimxAssignBoundaryConditionsGroup.cxx: In member function ‘void tkKWMimxAssignBoundaryConditionsGroup::UpdateBCVisibilityList()’: Modules/Meshing/BuildingBlock/vtkKWMimxAssignBoundaryConditionsGroup.cxx:2382: warning: declaration of ‘stepNumber’ shadows a previous local Modules/Meshing/BuildingBlock/vtkKWMimxAssignBoundaryConditionsGroup.cxx:2363: warning: shadowed declaration is here
Each unused variable warning produces 1 line:
Base/Logic/vtkSlicerApplicationLogic.cxx:1501: warning: unused variable ‘vnd’
Each suggest parentheses around && within || warning produces 1 line:
Base/GUI/vtkSlicerModelHierarchyWidget.cxx:980: warning: suggest parentheses around && within ||
Each comparison between signed and unsigned integer expressions warning produces 1 line:
Base/Logic/vtkImageConnectivity.cxx:630: warning: comparison between signed and unsigned integer expressions
Each deprecated conversion from string constant to char* warning produces 1 line:
Modules/QueryAtlas/vtkTextureFontManager.cxx:44: warning: deprecated conversion from string constant to ‘char*’
Each defined but not used warning produces 1 line:
Modules/QueryAtlas/vtkTextureText.h:59: warning: ‘sAutoLeading’ defined but not used
Each cast from type const char* to type char* casts away constness produces 1 line:
Modules/Tractography/Display/vtkSlicerFiberBundleLogic.cxx:131: warning: cast from type ‘const char*’ to type ‘char*’ casts away constness
Each format expects type , but argument has type produces 1 line:
Modules/QdecModule/vtkGDFReader.cxx:389: warning: format ‘%g’ expects type ‘float*’, but argument 3 has type ‘double*’
Each zero-length printf format string produces 1 line:
Base/GUI/vtkSlicerCacheAndDataIOManagerGUI.cxx:618: warning: zero-length printf format string
Dangerous Code Detected by Warnings
Warning | Code | Fixed Code |
---|---|---|
declaration of ‘BackgroundLayer’ shadows a member of 'this' |
void vtkSlicerSliceLogic::SetBackgroundLayer( vtkSlicerSliceLayerLogic *BackgroundLayer) { if (this->BackgroundLayer) { this->BackgroundLayer->SetAndObserveMRMLScene( NULL ); this->BackgroundLayer->Delete(); } this->BackgroundLayer = BackgroundLayer; ... |
void vtkSlicerSliceLogic::SetBackgroundLayer( vtkSlicerSliceLayerLogic *backgroundLayer) { if (this->BackgroundLayer) { this->BackgroundLayer->SetAndObserveMRMLScene( NULL ); this->BackgroundLayer->Delete(); } this->BackgroundLayer = backgroundLayer; ... |
statement has no effect |
//Enable/Disable navigation window this->EnableDisableNavButton; |
//Enable/Disable navigation window this->EnableDisableNavButton = NULL; |
comparison with string literal results in unspecified behaviour |
if ( this->GetUsername() == "" || this->GetPassword() == "") { |
if ( strcmp(this->GetUsername(),"") == 0 || strcmp(this->GetPassword(),"") == 0) { |
warning: format ‘%g’ expects type ‘float*’, but argument 3 has type ‘double*’ |
sscanf(line, "%*s %g", &this->ResidualFWHM); |
sscanf(line, "%*s %lg", &this->ResidualFWHM); |