kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #16803
[PATCH] IDF code: fixing problems reported by coverity
I have attached a patch to fix various problems reported by
coverity and tested to ensure that the patches did not break
the IDF exporter or idf2vrml tool. Here is a summary of
reported issues, including items which are false positives:
FIXED:
FILE: idf_parser.cpp
CID 102373 (#1 of 2): Resource leak (RESOURCE_LEAK)
CID 102442 (#1 of 1): Logically dead code (DEADCODE)
CID 102607 (#1 of 1): Use after free (USE_AFTER_FREE)
CID 102729 (#1 of 1): Uninitialized scalar field
FILE: idf_outlines.cpp
CID 102413 (#1 of 1): Logically dead code (DEADCODE)
Comment: inadvertent assignment when comparison
intended ('=' rather than '==')
CID 102828 (#1 of 1): Uninitialized scalar field
(UNINIT_CTOR)
Comment: 'height' is unused, now deleted. However
the bug also exposed a defect in the standards
compliance which has now been fixed.
FILE: idf_common.cpp
CID 102220 (#2 of 2): Using invalid iterator
(INVALIDATE_ITERATOR)
FILE: vrml_layer.cpp
CID 102555 (#2 of 2): Uninitialized scalar field
(UNINIT_CTOR)
FALSE POSITIVES:
FILE: idf_parser.cpp
CID 102184 (#1 of 1): Unchecked return value
CID 102198 (#1 of 1): Unchecked return value
CID 102201 (#1 of 1): Unchecked return value
CID 102210 (#1 of 1): Unchecked return value
CID 102214 (#1 of 1): Unchecked return value
FILE: idf_outlines.cpp
CID 102181 (#1 of 1): Unchecked return value
CID 102655 (#2-1 of 2): Not restoring ostream format
CID 102656 (#2-1 of 2): Not restoring ostream format
CID 102793 (#2-1 of 2): Not restoring ostream format
CID 102885 (#2-1 of 2): Not restoring ostream format
FILE: idf_common.cpp
CID 102183 (#1 of 1): Unchecked return value
CID 102191 (#1 of 1): Unchecked return value
CID 102556 (#1 of 1): Uninitialized scalar field
CID 102802 (#2-1 of 2): Not restoring ostream format
FILE: idf2vrml.cpp
CID 102602 (#1 of 1): Not restoring ostream format
- Cirilo
=== modified file 'utils/idftools/idf_common.cpp'
--- utils/idftools/idf_common.cpp 2014-06-01 16:55:53 +0000
+++ utils/idftools/idf_common.cpp 2015-02-16 06:44:26 +0000
@@ -950,7 +950,7 @@
PrintSeg( *bl );
#endif
aOutline.push( *bl );
- aLines.erase( bl );
+ bl = aLines.erase( bl );
}
continue;
@@ -982,7 +982,7 @@
printSeg( *bl );
#endif
aOutline.push( *bl );
- aLines.erase( bl );
+ bl = aLines.erase( bl );
}
continue;
=== modified file 'utils/idftools/idf_outlines.cpp'
--- utils/idftools/idf_outlines.cpp 2015-01-13 10:47:07 +0000
+++ utils/idftools/idf_outlines.cpp 2015-02-16 07:14:25 +0000
@@ -377,7 +377,7 @@
}
// verify winding of previous outline
- if( ( loopidx = 0 && !op->IsCCW() )
+ if( ( loopidx == 0 && !op->IsCCW() )
|| ( loopidx > 0 && op->IsCCW() ) )
{
ostringstream ostr;
@@ -2232,10 +2232,11 @@
setParent( aParent );
outlineType = OTLN_PLACE;
single = true;
- thickness = 0.0;
+ thickness = -1.0;
side = LYR_INVALID;
}
+
bool PLACE_OUTLINE::SetSide( IDF3::IDF_LAYER aSide )
{
#ifndef DISABLE_IDF_OWNERSHIP
@@ -2424,64 +2425,74 @@
throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
}
- if( !GetIDFString( iline, token, quoted, idx ) )
- {
- ostringstream ostr;
-
- ostr << "\n* invalid outline: " << GetOutlineTypeString( outlineType ) << "\n";
- ostr << "* violation: no height specified\n";
- ostr << "* line: '" << iline << "'\n";
- ostr << "* file position: " << pos;
-
- throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
- }
-
- std::stringstream teststr;
- teststr << token;
-
- teststr >> thickness;
- if( teststr.fail() )
- {
- ostringstream ostr;
-
- ostr << "\n* invalid outline: " << GetOutlineTypeString( outlineType ) << "\n";
- ostr << "* violation: invalid height\n";
- ostr << "* line: '" << iline << "'\n";
- ostr << "* file position: " << pos;
-
- throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
- }
-
- if( thickness < 0.0 )
- {
- ostringstream ostr;
-
- ostr << "\n* invalid outline: " << GetOutlineTypeString( outlineType ) << "\n";
- ostr << "* violation: thickness < 0\n";
- ostr << "* line: '" << iline << "'\n";
- ostr << "* file position: " << pos;
-
- throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
- }
-
- if( unit == UNIT_THOU )
- {
- thickness *= IDF_THOU_TO_MM;
- }
- else if( ( aIdfVersion == IDF_V2 ) && ( unit == UNIT_TNM ) )
- {
- thickness *= IDF_TNM_TO_MM;
- }
- else if( unit != UNIT_MM )
- {
- ostringstream ostr;
- ostr << "\n* BUG: invalid UNIT type: " << unit;
-
- throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
- }
-
- if( thickness < 0.0 )
- thickness = 0.0;
+ if( GetIDFString( iline, token, quoted, idx ) )
+ {
+ std::stringstream teststr;
+ teststr << token;
+
+ teststr >> thickness;
+
+ if( teststr.fail() )
+ {
+ ostringstream ostr;
+
+ ostr << "\n* invalid outline: " << GetOutlineTypeString( outlineType ) << "\n";
+ ostr << "* violation: invalid height\n";
+ ostr << "* line: '" << iline << "'\n";
+ ostr << "* file position: " << pos;
+
+ throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
+ }
+
+ if( thickness < 0.0 )
+ {
+ ostringstream ostr;
+
+ ostr << "\n* invalid outline: " << GetOutlineTypeString( outlineType ) << "\n";
+ ostr << "* violation: thickness < 0\n";
+ ostr << "* line: '" << iline << "'\n";
+ ostr << "* file position: " << pos;
+
+ throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
+ }
+
+ if( unit == UNIT_THOU )
+ {
+ thickness *= IDF_THOU_TO_MM;
+ }
+ else if( ( aIdfVersion == IDF_V2 ) && ( unit == UNIT_TNM ) )
+ {
+ thickness *= IDF_TNM_TO_MM;
+ }
+ else if( unit != UNIT_MM )
+ {
+ ostringstream ostr;
+ ostr << "\n* BUG: invalid UNIT type: " << unit;
+
+ throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
+ }
+
+ if( thickness < 0.0 )
+ thickness = 0.0;
+
+ }
+ else
+ {
+ // for OTLN_PLACE, thickness may be omitted, but is required for OTLN_PLACE_KEEPOUT
+ if( outlineType == OTLN_PLACE_KEEPOUT )
+ {
+ ostringstream ostr;
+
+ ostr << "\n* invalid outline: " << GetOutlineTypeString( outlineType ) << "\n";
+ ostr << "* violation: missing thickness\n";
+ ostr << "* line: '" << iline << "'\n";
+ ostr << "* file position: " << pos;
+
+ throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
+ }
+
+ thickness = -1.0;
+ }
}
else
{
@@ -2574,12 +2585,20 @@
break;
}
- aBoardFile << " ";
-
- if( unit != UNIT_THOU )
- aBoardFile << setiosflags(ios::fixed) << setprecision(5) << thickness << "\n";
+ // thickness is optional for OTLN_PLACE, but mandatory for OTLN_PLACE_KEEPOUT
+ if( thickness < 0.0 && outlineType == OTLN_PLACE_KEEPOUT)
+ {
+ aBoardFile << "\n";
+ }
else
- aBoardFile << setiosflags(ios::fixed) << setprecision(1) << (thickness / IDF_THOU_TO_MM) << "\n";
+ {
+ aBoardFile << " ";
+
+ if( unit != UNIT_THOU )
+ aBoardFile << setiosflags(ios::fixed) << setprecision(5) << thickness << "\n";
+ else
+ aBoardFile << setiosflags(ios::fixed) << setprecision(1) << (thickness / IDF_THOU_TO_MM) << "\n";
+ }
// write RECORD 3
writeOutlines( aBoardFile );
=== modified file 'utils/idftools/idf_outlines.h'
--- utils/idftools/idf_outlines.h 2014-06-01 16:55:53 +0000
+++ utils/idftools/idf_outlines.h 2015-02-16 06:12:07 +0000
@@ -498,7 +498,6 @@
protected:
IDF3::IDF_LAYER side; // Board Side [TOP/BOTTOM/BOTH ONLY] (IDF spec)
- double height; // Max Height (IDF spec)
public:
PLACE_OUTLINE( IDF3_BOARD* aParent );
=== modified file 'utils/idftools/idf_parser.cpp'
--- utils/idftools/idf_parser.cpp 2015-01-13 10:47:07 +0000
+++ utils/idftools/idf_parser.cpp 2015-02-16 06:47:04 +0000
@@ -1050,9 +1050,7 @@
{
val = true;
delete *itS;
- drills.erase( itS );
- itS = drills.begin();
- itE = drills.end();
+ itS = drills.erase( itS );
continue;
}
++itS;
@@ -1341,6 +1339,7 @@
brdFileVersion = 0;
libFileVersion = 0;
iRefDes = 0;
+ unit = UNIT_MM;
// unlike other outlines which are created as necessary,
// the board outline always exists and its parent must
@@ -2002,14 +2001,13 @@
if( olnOther.insert( pair<string, OTHER_OUTLINE*>(op->GetOutlineIdentifier(), op) ).second == false )
{
- delete op;
-
ostringstream ostr;
ostr << "invalid IDF file\n";
ostr << "* Violation of specification. Non-unique ID in OTHER_OUTLINE '";
ostr << op->GetOutlineIdentifier() << "'\n";
ostr << "* line: '" << iline << "'\n";
ostr << "* pos: " << pos;
+ delete op;
throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
}
@@ -2491,6 +2489,7 @@
ostr << "* Violation of specification: multiple outlines have the same GEOM and PART name\n";
ostr << "* line: '" << iline << "'\n";
ostr << "* pos: " << pos;
+ delete pout;
throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
}
@@ -2504,6 +2503,7 @@
ostr << "* Expecting .ELECTRICAL or .MECHANICAL, got '" << token << "'\n";
ostr << "* line: '" << iline << "'\n";
ostr << "* pos: " << pos;
+ delete pout;
throw( IDF_ERROR( __FILE__, __FUNCTION__, __LINE__, ostr.str() ) );
}
@@ -3429,10 +3429,6 @@
switch( keyo )
{
- case UNOWNED:
- ostr << "UNOWNED";
- break;
-
case ECAD:
ostr << "ECAD";
break;
=== modified file 'utils/idftools/vrml_layer.cpp'
--- utils/idftools/vrml_layer.cpp 2014-11-15 13:43:23 +0000
+++ utils/idftools/vrml_layer.cpp 2015-02-16 06:37:43 +0000
@@ -178,6 +178,8 @@
fix = false;
Fault = false;
idx = 0;
+ hidx = 0;
+ eidx = 0;
ord = 0;
glcmd = 0;
pholes = NULL;
=== modified file 'utils/idftools/vrml_layer.h'
--- utils/idftools/vrml_layer.h 2014-11-15 13:43:23 +0000
+++ utils/idftools/vrml_layer.h 2015-02-16 06:38:46 +0000
@@ -100,7 +100,6 @@
bool fix; // when true, no more vertices may be added by the user
int idx; // vertex index (number of contained vertices)
int ord; // vertex order (number of ordered vertices)
- unsigned int idxout; // outline index to first point in 3D outline
std::vector<VERTEX_3D*> vertices; // vertices of all contours
std::vector<std::list<int>*> contours; // lists of vertices for each contour
std::vector<bool>pth; // indicates whether a 'contour' is a PTH or not
Follow ups
