Sim: Fix crash when a field tries to set an inexistent parameter

Refactored SetParamValue() and friends to use exceptions instead of
return values.
This commit is contained in:
Mikolaj Wielgus 2022-10-12 04:26:16 +02:00
parent 6ae333a116
commit c857e533a9
10 changed files with 62 additions and 52 deletions

View File

@ -840,22 +840,21 @@ const SIM_MODEL::PARAM& SIM_MODEL::GetBaseParam( unsigned aParamIndex ) const
} }
bool SIM_MODEL::SetParamValue( int aParamIndex, const SIM_VALUE& aValue ) void SIM_MODEL::SetParamValue( int aParamIndex, const SIM_VALUE& aValue )
{ {
*m_params.at( aParamIndex ).value = aValue; *m_params.at( aParamIndex ).value = aValue;
return true;
} }
bool SIM_MODEL::SetParamValue( int aParamIndex, const std::string& aValue, void SIM_MODEL::SetParamValue( int aParamIndex, const std::string& aValue,
SIM_VALUE::NOTATION aNotation ) SIM_VALUE::NOTATION aNotation )
{ {
const SIM_VALUE& value = *GetParam( aParamIndex ).value; const SIM_VALUE& value = *GetParam( aParamIndex ).value;
return SetParamValue( aParamIndex, *SIM_VALUE::Create( value.GetType(), aValue, aNotation ) ); SetParamValue( aParamIndex, *SIM_VALUE::Create( value.GetType(), aValue, aNotation ) );
} }
bool SIM_MODEL::SetParamValue( const std::string& aParamName, const SIM_VALUE& aValue ) void SIM_MODEL::SetParamValue( const std::string& aParamName, const SIM_VALUE& aValue )
{ {
std::vector<std::reference_wrapper<const PARAM>> params = GetParams(); std::vector<std::reference_wrapper<const PARAM>> params = GetParams();
@ -866,18 +865,28 @@ bool SIM_MODEL::SetParamValue( const std::string& aParamName, const SIM_VALUE& a
} ); } );
if( it == params.end() ) if( it == params.end() )
return false; {
THROW_IO_ERROR( wxString::Format( "Could not find a model parameter named '%s'",
aParamName ) );
}
SetParamValue( static_cast<int>( it - params.begin() ), aValue ); SetParamValue( static_cast<int>( it - params.begin() ), aValue );
return true;
} }
bool SIM_MODEL::SetParamValue( const std::string& aParamName, const std::string& aValue, void SIM_MODEL::SetParamValue( const std::string& aParamName, const std::string& aValue,
SIM_VALUE::NOTATION aNotation ) SIM_VALUE::NOTATION aNotation )
{ {
const PARAM* param = FindParam( aParamName );
if( !param )
{
THROW_IO_ERROR( wxString::Format( "Could not find a model parameter named '%s'",
aParamName ) );
}
const SIM_VALUE& value = *FindParam( aParamName )->value; const SIM_VALUE& value = *FindParam( aParamName )->value;
return SetParamValue( aParamName, *SIM_VALUE::Create( value.GetType(), aValue, aNotation ) ); SetParamValue( aParamName, *SIM_VALUE::Create( value.GetType(), aValue, aNotation ) );
} }

View File

@ -544,11 +544,11 @@ public:
const PARAM& GetBaseParam( unsigned aParamIndex ) const; // Always return base parameter if it exists. const PARAM& GetBaseParam( unsigned aParamIndex ) const; // Always return base parameter if it exists.
virtual bool SetParamValue( int aParamIndex, const SIM_VALUE& aValue ); virtual void SetParamValue( int aParamIndex, const SIM_VALUE& aValue );
bool SetParamValue( int aParamIndex, const std::string& aValue, void SetParamValue( int aParamIndex, const std::string& aValue,
SIM_VALUE::NOTATION aNotation = SIM_VALUE::NOTATION::SI ); SIM_VALUE::NOTATION aNotation = SIM_VALUE::NOTATION::SI );
bool SetParamValue( const std::string& aParamName, const SIM_VALUE& aValue ); void SetParamValue( const std::string& aParamName, const SIM_VALUE& aValue );
bool SetParamValue( const std::string& aParamName, const std::string& aValue, void SetParamValue( const std::string& aParamName, const std::string& aValue,
SIM_VALUE::NOTATION aNotation = SIM_VALUE::NOTATION::SI ); SIM_VALUE::NOTATION aNotation = SIM_VALUE::NOTATION::SI );
bool HasOverrides() const; bool HasOverrides() const;

View File

@ -81,7 +81,8 @@ SIM_MODEL_NGSPICE::SIM_MODEL_NGSPICE( TYPE aType ) :
} }
bool SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, const std::string& aParamValue, void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName,
const std::string& aParamValue,
SIM_VALUE_GRAMMAR::NOTATION aNotation ) SIM_VALUE_GRAMMAR::NOTATION aNotation )
{ {
std::string paramName = boost::to_lower_copy( aParamName ); std::string paramName = boost::to_lower_copy( aParamName );
@ -89,24 +90,24 @@ bool SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, co
// "level" and "version" are not really parameters - they're part of the type - so silently // "level" and "version" are not really parameters - they're part of the type - so silently
// ignore them. // ignore them.
if( paramName == "level" || paramName == "version" ) if( paramName == "level" || paramName == "version" )
return true; return;
// Also ignore "type" parameter, because Ngspice does that too. // Also ignore "type" parameter, because Ngspice does that too.
if( paramName == "type" ) if( paramName == "type" )
return true; return;
if( GetDeviceType() == DEVICE_TYPE_::NPN || GetDeviceType() == DEVICE_TYPE_::PNP ) if( GetDeviceType() == DEVICE_TYPE_::NPN || GetDeviceType() == DEVICE_TYPE_::PNP )
{ {
// Ignore the purely informative LTspice-specific parameters "mfg", "icrating", "vceo". // Ignore the purely informative LTspice-specific parameters "mfg", "icrating", "vceo".
if( paramName == "mfg" || paramName == "icrating" || paramName == "vceo" ) if( paramName == "mfg" || paramName == "icrating" || paramName == "vceo" )
return true; return;
// Ignore unused parameters. // Ignore unused parameters.
if( paramName == "bvcbo" || paramName == "nbvcbo" if( paramName == "bvcbo" || paramName == "nbvcbo"
|| paramName == "tbvcbo1" || paramName == "tbvcbo2" || paramName == "tbvcbo1" || paramName == "tbvcbo2"
|| paramName == "bvbe" || paramName == "ibvbe" || paramName == "nbvbe" ) || paramName == "bvbe" || paramName == "ibvbe" || paramName == "nbvbe" )
{ {
return true; return;
} }
} }
@ -125,8 +126,10 @@ bool SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, co
} ); } );
if( paramIt != params.end() ) if( paramIt != params.end() )
return SIM_MODEL::SetParamValue( static_cast<int>( paramIt - params.begin() ), aParamValue, {
aNotation ); SIM_MODEL::SetParamValue( static_cast<int>( paramIt - params.begin() ), aParamValue, aNotation );
return;
}
// One Spice param can have multiple names, we need to take this into account. // One Spice param can have multiple names, we need to take this into account.
@ -140,7 +143,11 @@ bool SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, co
} ); } );
if( ngspiceParamIt == ngspiceParams.end() ) if( ngspiceParamIt == ngspiceParams.end() )
return false; {
THROW_IO_ERROR( wxString::Format( "Failed to set parameter '%s' to value '%s'",
aParamName,
aParamValue ) );
}
// We obtain the id of the Ngspice param that is to be set. // We obtain the id of the Ngspice param that is to be set.
@ -154,10 +161,13 @@ bool SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, co
} ); } );
if( paramIt == params.end() ) if( paramIt == params.end() )
return false; {
THROW_IO_ERROR( wxString::Format( "Failed to set parameter '%s' to value '%s'",
aParamName,
aParamValue ) );
}
return SIM_MODEL::SetParamValue( static_cast<int>( paramIt - params.begin() ), aParamValue, SIM_MODEL::SetParamValue( static_cast<int>( paramIt - params.begin() ), aParamValue, aNotation );
aNotation );
} }

View File

@ -46,7 +46,7 @@ public:
SIM_MODEL_NGSPICE( TYPE aType ); SIM_MODEL_NGSPICE( TYPE aType );
bool SetParamFromSpiceCode( const std::string& aParamName, const std::string& aParamValue, void SetParamFromSpiceCode( const std::string& aParamName, const std::string& aParamValue,
SIM_VALUE_GRAMMAR::NOTATION aNotation ) override; SIM_VALUE_GRAMMAR::NOTATION aNotation ) override;
// Protected because it's accessed by QA tests. // Protected because it's accessed by QA tests.

View File

@ -225,7 +225,7 @@ void SIM_MODEL_SOURCE::WriteDataLibFields( std::vector<LIB_FIELD>& aFields ) con
} }
bool SIM_MODEL_SOURCE::SetParamValue( int aParamIndex, const SIM_VALUE& aValue ) void SIM_MODEL_SOURCE::SetParamValue( int aParamIndex, const SIM_VALUE& aValue )
{ {
// Sources are special. All preceding parameter values must be filled. If they are not, fill // Sources are special. All preceding parameter values must be filled. If they are not, fill
// them out automatically. If a value is nulled, delete everything after it. // them out automatically. If a value is nulled, delete everything after it.

View File

@ -66,7 +66,7 @@ public:
void WriteDataSchFields( std::vector<SCH_FIELD>& aFields ) const override; void WriteDataSchFields( std::vector<SCH_FIELD>& aFields ) const override;
void WriteDataLibFields( std::vector<LIB_FIELD>& aFields ) const override; void WriteDataLibFields( std::vector<LIB_FIELD>& aFields ) const override;
bool SetParamValue( int aParamIndex, const SIM_VALUE& aValue ) override; void SetParamValue( int aParamIndex, const SIM_VALUE& aValue ) override;
bool HasAutofill() const override { return true; } bool HasAutofill() const override { return true; }

View File

@ -89,19 +89,18 @@ SIM_MODEL_SPICE::SIM_MODEL_SPICE( TYPE aType,
} }
bool SIM_MODEL_SPICE::SetParamValue( int aParamIndex, const SIM_VALUE& aValue ) void SIM_MODEL_SPICE::SetParamValue( int aParamIndex, const SIM_VALUE& aValue )
{ {
// Models sourced from a library are immutable.
if( m_spiceCode != "" ) if( m_spiceCode != "" )
return false; THROW_IO_ERROR( "Could not change model parameters: library models are immutable" );
return SIM_MODEL::SetParamValue( aParamIndex, aValue ); SIM_MODEL::SetParamValue( aParamIndex, aValue );
} }
bool SIM_MODEL_SPICE::SetParamFromSpiceCode( const std::string& aParamName, void SIM_MODEL_SPICE::SetParamFromSpiceCode( const std::string& aParamName,
const std::string& aParamValue, const std::string& aParamValue,
SIM_VALUE_GRAMMAR::NOTATION aNotation ) SIM_VALUE_GRAMMAR::NOTATION aNotation )
{ {
return SIM_MODEL::SetParamValue( aParamName, aParamValue, aNotation ); SIM_MODEL::SetParamValue( aParamName, aParamValue, aNotation );
} }

View File

@ -58,9 +58,9 @@ public:
std::unique_ptr<SPICE_MODEL_PARSER> aSpiceModelParser ); std::unique_ptr<SPICE_MODEL_PARSER> aSpiceModelParser );
protected: protected:
bool SetParamValue( int aParamIndex, const SIM_VALUE& aValue ) override; void SetParamValue( int aParamIndex, const SIM_VALUE& aValue ) override;
virtual bool SetParamFromSpiceCode( const std::string& aParamName, virtual void SetParamFromSpiceCode( const std::string& aParamName,
const std::string& aParamValue, const std::string& aParamValue,
SIM_VALUE_GRAMMAR::NOTATION aNotation SIM_VALUE_GRAMMAR::NOTATION aNotation
= SIM_VALUE_GRAMMAR::NOTATION::SPICE ); = SIM_VALUE_GRAMMAR::NOTATION::SPICE );

View File

@ -384,8 +384,14 @@ bool SIM_STRING_PROPERTY::StringToValue( wxVariant& aVariant, const wxString& aT
// TODO: Don't use string comparison. // TODO: Don't use string comparison.
if( m_model->GetBaseModel() && ( aText == "" || aText == baseParamValue ) ) if( m_model->GetBaseModel() && ( aText == "" || aText == baseParamValue ) )
{ {
if( !m_model->SetParamValue( m_paramIndex, "" ) ) // Nullify. try
{
m_model->SetParamValue( m_paramIndex, "" ); // Nullify.
}
catch( const IO_ERROR& )
{
return false; return false;
}
aVariant = baseParamValue; // Use the inherited value (if it exists) if null. aVariant = baseParamValue; // Use the inherited value (if it exists) if null.
} }

View File

@ -211,14 +211,7 @@ void SPICE_MODEL_PARSER::ReadModel( const SIM_LIBRARY_SPICE& aLibrary,
{ {
wxASSERT( paramName != "" ); wxASSERT( paramName != "" );
if( !m_model.SetParamFromSpiceCode( paramName, subnode->string() ) ) m_model.SetParamFromSpiceCode( paramName, subnode->string() );
{
THROW_IO_ERROR( wxString::Format(
_( "Failed to set parameter '%s' to '%s' in model '%s'" ),
paramName,
subnode->string(),
modelName ) );
}
} }
else else
{ {
@ -249,14 +242,7 @@ void SPICE_MODEL_PARSER::ReadModel( const SIM_LIBRARY_SPICE& aLibrary,
{ {
wxASSERT( paramName != "" ); wxASSERT( paramName != "" );
if( !m_model.SetParamFromSpiceCode( paramName, subnode->string() ) ) m_model.SetParamFromSpiceCode( paramName, subnode->string() );
{
THROW_IO_ERROR( wxString::Format(
_( "Failed to set parameter '%s' to '%s' in model '%s'" ),
paramName,
subnode->string(),
modelName ) );
}
} }
else else
{ {