From 9d348d7e6d97ef5edea68a2291de3ec1c0977b66 Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Tue, 6 Jul 2021 13:32:34 -0400 Subject: [PATCH] Coverity issue fixes. --- common/env_vars.cpp | 8 +- common/libeval/numeric_evaluator.cpp | 119 ++++++++++--------- eeschema/sch_symbol.cpp | 24 ++-- pcbnew/exporters/export_vrml.cpp | 6 +- qa/pcbnew/drc/test_drc_courtyard_invalid.cpp | 8 +- 5 files changed, 85 insertions(+), 80 deletions(-) diff --git a/common/env_vars.cpp b/common/env_vars.cpp index 711bd4cc20..481c2701b2 100644 --- a/common/env_vars.cpp +++ b/common/env_vars.cpp @@ -115,8 +115,6 @@ wxString ENV_VAR::LookUpEnvVarHelp( const wxString& aEnvVar ) template<> OPT ENV_VAR::GetEnvVar( const wxString& aEnvVarName ) { - OPT optValue = NULLOPT; - wxString env; if( wxGetEnv( aEnvVarName, &env ) ) @@ -124,12 +122,10 @@ OPT ENV_VAR::GetEnvVar( const wxString& aEnvVarName ) double value; if( env.ToDouble( &value ) ) - { - optValue = value; - } + return value; } - return optValue; + return NULLOPT; } diff --git a/common/libeval/numeric_evaluator.cpp b/common/libeval/numeric_evaluator.cpp index c0d586f8bc..2ba75f205b 100644 --- a/common/libeval/numeric_evaluator.cpp +++ b/common/libeval/numeric_evaluator.cpp @@ -1,21 +1,22 @@ /* - This file is part of libeval, a simple math expression evaluator - - Copyright (C) 2017 Michael Geselbracht, mgeselbracht3@gmail.com - - This program is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program. If not, see . -*/ + * This file is part of libeval, a simple math expression evaluator + * + * Copyright (C) 2017 Michael Geselbracht, mgeselbracht3@gmail.com + * Copyright (C) 2021 KiCad Developers, see AUTHORS.txt for contributors. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ #include #include @@ -55,10 +56,10 @@ NUMERIC_EVALUATOR::NUMERIC_EVALUATOR( EDA_UNITS aUnits ) switch( aUnits ) { - case EDA_UNITS::MILLIMETRES: m_defaultUnits = Unit::MM; break; - case EDA_UNITS::MILS: m_defaultUnits = Unit::Mil; break; - case EDA_UNITS::INCHES: m_defaultUnits = Unit::Inch; break; - default: m_defaultUnits = Unit::MM; break; + case EDA_UNITS::MILLIMETRES: m_defaultUnits = Unit::MM; break; + case EDA_UNITS::MILS: m_defaultUnits = Unit::Mil; break; + case EDA_UNITS::INCHES: m_defaultUnits = Unit::Inch; break; + default: m_defaultUnits = Unit::MM; break; } } @@ -176,6 +177,7 @@ NUMERIC_EVALUATOR::Token NUMERIC_EVALUATOR::getToken() retval.token = ENDS; retval.value.dValue = 0; + retval.value.valid = false; if( m_token.token == nullptr ) return retval; @@ -203,18 +205,20 @@ NUMERIC_EVALUATOR::Token NUMERIC_EVALUATOR::getToken() m_token.token[ idx++ ] = ch; - if( isDecimalSeparator( ch )) + if( isDecimalSeparator( ch ) ) haveSeparator = true; ch = m_token.input[ ++m_token.pos ]; - } while( isdigit( ch ) || isDecimalSeparator( ch )); + } while( isdigit( ch ) || isDecimalSeparator( ch ) ); m_token.token[ idx ] = 0; // Ensure that the systems decimal separator is used for( int i = strlen( m_token.token ); i; i-- ) - if( isDecimalSeparator( m_token.token[ i - 1 ] )) + { + if( isDecimalSeparator( m_token.token[ i - 1 ] ) ) m_token.token[ i - 1 ] = m_localeDecimalSeparator; + } }; // Lamda: Get unit for current token. @@ -232,31 +236,33 @@ NUMERIC_EVALUATOR::Token NUMERIC_EVALUATOR::getToken() const char* cptr = &m_token.input[ m_token.pos ]; const auto sizeLeft = m_token.inputLen - m_token.pos; - if( sizeLeft >= 2 && ch == 'm' && cptr[ 1 ] == 'm' && !isalnum( cptr[ 2 ] )) + if( sizeLeft >= 2 && ch == 'm' && cptr[ 1 ] == 'm' && !isalnum( cptr[ 2 ] ) ) { m_token.pos += 2; return Unit::MM; } - if( sizeLeft >= 2 && ch == 'c' && cptr[ 1 ] == 'm' && !isalnum( cptr[ 2 ] )) + if( sizeLeft >= 2 && ch == 'c' && cptr[ 1 ] == 'm' && !isalnum( cptr[ 2 ] ) ) { m_token.pos += 2; return Unit::CM; } - if( sizeLeft >= 2 && ch == 'i' && cptr[ 1 ] == 'n' && !isalnum( cptr[ 2 ] )) + if( sizeLeft >= 2 && ch == 'i' && cptr[ 1 ] == 'n' && !isalnum( cptr[ 2 ] ) ) { m_token.pos += 2; return Unit::Inch; } - if( sizeLeft >= 3 && ch == 'm' && cptr[ 1 ] == 'i' && cptr[ 2 ] == 'l' && !isalnum( cptr[ 3 ] )) + if( sizeLeft >= 3 && ch == 'm' && cptr[ 1 ] == 'i' && cptr[ 2 ] == 'l' + && !isalnum( cptr[ 3 ] ) ) { m_token.pos += 3; return Unit::Mil; } - if( sizeLeft >= 4 && ch == 't' && cptr[ 1 ] == 'h' && cptr[ 2 ] == 'o' && cptr[ 3 ] == 'u' && !isalnum( cptr[ 4 ] )) + if( sizeLeft >= 4 && ch == 't' && cptr[ 1 ] == 'h' && cptr[ 2 ] == 'o' + && cptr[ 3 ] == 'u' && !isalnum( cptr[ 4 ] ) ) { m_token.pos += 4; return Unit::Mil; @@ -300,37 +306,38 @@ NUMERIC_EVALUATOR::Token NUMERIC_EVALUATOR::getToken() // The factor is assigned to the terminal UNIT. The actual // conversion is done within a parser action. retval.token = UNIT; + if( m_defaultUnits == Unit::MM ) { switch( convertFrom ) { - case Unit::Inch :retval.value.dValue = 25.4; break; - case Unit::Mil :retval.value.dValue = 25.4 / 1000.0; break; - case Unit::MM :retval.value.dValue = 1.0; break; - case Unit::CM :retval.value.dValue = 10.0; break; - case Unit::Invalid :break; + case Unit::Inch: retval.value.dValue = 25.4; break; + case Unit::Mil: retval.value.dValue = 25.4 / 1000.0; break; + case Unit::MM: retval.value.dValue = 1.0; break; + case Unit::CM: retval.value.dValue = 10.0; break; + case Unit::Invalid: break; } } else if( m_defaultUnits == Unit::Inch ) { switch( convertFrom ) { - case Unit::Inch :retval.value.dValue = 1.0; break; - case Unit::Mil :retval.value.dValue = 1.0 / 1000.0; break; - case Unit::MM :retval.value.dValue = 1.0 / 25.4; break; - case Unit::CM :retval.value.dValue = 1.0 / 2.54; break; - case Unit::Invalid :break; + case Unit::Inch: retval.value.dValue = 1.0; break; + case Unit::Mil: retval.value.dValue = 1.0 / 1000.0; break; + case Unit::MM: retval.value.dValue = 1.0 / 25.4; break; + case Unit::CM: retval.value.dValue = 1.0 / 2.54; break; + case Unit::Invalid: break; } } else if( m_defaultUnits == Unit::Mil ) { switch( convertFrom ) { - case Unit::Inch :retval.value.dValue = 1.0 * 1000.0; break; - case Unit::Mil :retval.value.dValue = 1.0; break; - case Unit::MM :retval.value.dValue = 1000.0 / 25.4; break; - case Unit::CM :retval.value.dValue = 1000.0 / 2.54; break; - case Unit::Invalid :break; + case Unit::Inch: retval.value.dValue = 1.0 * 1000.0; break; + case Unit::Mil: retval.value.dValue = 1.0; break; + case Unit::MM: retval.value.dValue = 1000.0 / 25.4; break; + case Unit::CM: retval.value.dValue = 1000.0 / 2.54; break; + case Unit::Invalid: break; } } } @@ -346,7 +353,7 @@ NUMERIC_EVALUATOR::Token NUMERIC_EVALUATOR::getToken() retval.token = VAR; size_t bytesToCopy = cptr - &m_token.input[ m_token.pos ]; - if( bytesToCopy >= sizeof( retval.value.text )) + if( bytesToCopy >= sizeof( retval.value.text ) ) bytesToCopy = sizeof( retval.value.text ) - 1; strncpy( retval.value.text, &m_token.input[ m_token.pos ], bytesToCopy ); @@ -358,19 +365,23 @@ NUMERIC_EVALUATOR::Token NUMERIC_EVALUATOR::getToken() // Single char tokens switch( ch ) { - case '+' :retval.token = PLUS; break; - case '-' :retval.token = MINUS; break; - case '*' :retval.token = MULT; break; - case '/' :retval.token = DIVIDE; break; - case '(' :retval.token = PARENL; break; - case ')' :retval.token = PARENR; break; - case '=' :retval.token = ASSIGN; break; - case ';' :retval.token = SEMCOL; break; - default :m_parseError = true; break; /* invalid character */ + case '+': retval.token = PLUS; break; + case '-': retval.token = MINUS; break; + case '*': retval.token = MULT; break; + case '/': retval.token = DIVIDE; break; + case '(': retval.token = PARENL; break; + case ')': retval.token = PARENR; break; + case '=': retval.token = ASSIGN; break; + case ';': retval.token = SEMCOL; break; + default: m_parseError = true; break; /* invalid character */ } + m_token.pos++; } + if( !m_parseError ) + retval.value.valid = true; + return retval; } diff --git a/eeschema/sch_symbol.cpp b/eeschema/sch_symbol.cpp index 536d41815d..68264c426c 100644 --- a/eeschema/sch_symbol.cpp +++ b/eeschema/sch_symbol.cpp @@ -934,15 +934,18 @@ bool SCH_SYMBOL::ResolveTextVar( wxString* token, int aDepth ) const { SCHEMATIC* schematic = Schematic(); + // SCH_SYMOL object has no context outside a schematic. + wxCHECK( schematic, false ); + for( int i = 0; i < MANDATORY_FIELDS; ++i ) { if( token->IsSameAs( m_fields[ i ].GetCanonicalName().Upper() ) ) { - if( i == REFERENCE_FIELD && schematic ) + if( i == REFERENCE_FIELD ) *token = GetRef( &schematic->CurrentSheet(), true ); - else if( i == VALUE_FIELD && schematic ) + else if( i == VALUE_FIELD ) *token = GetValue( &schematic->CurrentSheet(), true ); - else if( i == FOOTPRINT_FIELD && schematic ) + else if( i == FOOTPRINT_FIELD ) *token = GetFootprint( &schematic->CurrentSheet(), true ); else *token = m_fields[ i ].GetShownText( aDepth + 1 ); @@ -978,10 +981,7 @@ bool SCH_SYMBOL::ResolveTextVar( wxString* token, int aDepth ) const { wxString footprint; - if( schematic ) - footprint = GetFootprint( &schematic->CurrentSheet(), true ); - else - footprint = m_fields[ FOOTPRINT_FIELD ].GetShownText(); + footprint = GetFootprint( &schematic->CurrentSheet(), true ); wxArrayString parts = wxSplit( footprint, ':' ); @@ -992,10 +992,7 @@ bool SCH_SYMBOL::ResolveTextVar( wxString* token, int aDepth ) const { wxString footprint; - if( schematic ) - footprint = GetFootprint( &schematic->CurrentSheet(), true ); - else - footprint = m_fields[ FOOTPRINT_FIELD ].GetShownText(); + footprint = GetFootprint( &schematic->CurrentSheet(), true ); wxArrayString parts = wxSplit( footprint, ':' ); @@ -1006,10 +1003,7 @@ bool SCH_SYMBOL::ResolveTextVar( wxString* token, int aDepth ) const { int unit; - if( schematic ) - unit = GetUnitSelection( &schematic->CurrentSheet() ); - else - unit = GetUnit(); + unit = GetUnitSelection( &schematic->CurrentSheet() ); *token = LIB_SYMBOL::SubReference( unit ); return true; diff --git a/pcbnew/exporters/export_vrml.cpp b/pcbnew/exporters/export_vrml.cpp index 9f1a8400a9..689b6a55f3 100644 --- a/pcbnew/exporters/export_vrml.cpp +++ b/pcbnew/exporters/export_vrml.cpp @@ -882,11 +882,11 @@ void EXPORTER_PCB_VRML::ExportVrmlFootprint( FOOTPRINT* aFootprint, std::ostream // only write a rotation if it is >= 0.1 deg if( std::abs( rot[3] ) > 0.0001745 ) { - (*aOutputFile) << " rotation " << std::setprecision( 5 ); + (*aOutputFile) << " rotation " << aOutputFile->precision( 5 ); (*aOutputFile) << rot[0] << " " << rot[1] << " " << rot[2] << " " << rot[3] << "\n"; } - (*aOutputFile) << " translation " << std::setprecision( m_precision ); + (*aOutputFile) << " translation " << aOutputFile->precision( m_precision ); (*aOutputFile) << trans.x << " "; (*aOutputFile) << trans.y << " "; (*aOutputFile) << trans.z << "\n"; @@ -938,7 +938,7 @@ void EXPORTER_PCB_VRML::ExportVrmlFootprint( FOOTPRINT* aFootprint, std::ostream ++sM; } - std::setprecision( old_precision ); + aOutputFile->precision( old_precision ); } diff --git a/qa/pcbnew/drc/test_drc_courtyard_invalid.cpp b/qa/pcbnew/drc/test_drc_courtyard_invalid.cpp index 3e66141235..fb7ec5f142 100644 --- a/qa/pcbnew/drc/test_drc_courtyard_invalid.cpp +++ b/qa/pcbnew/drc/test_drc_courtyard_invalid.cpp @@ -236,8 +236,6 @@ static bool InvalidMatchesExpected( BOARD& aBoard, const PCB_MARKER& aMarker, auto reporter = std::static_pointer_cast( aMarker.GetRCItem() ); const FOOTPRINT* item_a = dynamic_cast( aBoard.GetItem( reporter->GetMainItemID() ) ); - BOOST_CHECK( item_a != nullptr ); - // This one is more than just a mismatch! if( reporter->GetAuxItemID() != niluuid ) { @@ -245,6 +243,12 @@ static bool InvalidMatchesExpected( BOARD& aBoard, const PCB_MARKER& aMarker, return false; } + if( item_a == nullptr ) + { + BOOST_ERROR( "Could not get board DRC item." ); + return false; + } + if( item_a->GetReference() != aInvalid.m_refdes ) return false;