haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix CADSTAR importer memory leaks 

We were leaking the xml tree when throwing exceptions


(cherry picked from commit b03366c9e8)
This commit is contained in:
Roberto Fernandez Bautista 2023-02-27 23:55:20 +01:00
parent 46c8985661
commit c96f0f2396
6 changed files with 60 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
common/plugins/cadstar/cadstar_archive_parser.cpp
View File

@ -2428,7 +2428,9 @@ XNODE* CADSTAR_ARCHIVE_PARSER::LoadArchiveFile( const wxString& aFileName,
const wxString& aFileTypeIdentifier, PROGRESS_REPORTER* aProgressReporter ) const wxString& aFileTypeIdentifier, PROGRESS_REPORTER* aProgressReporter )
{ {
KEYWORD emptyKeywords[1] = {}; KEYWORD emptyKeywords[1] = {};
XNODE * iNode = nullptr, *cNode = nullptr; XNODE* rootNode = nullptr;
XNODE* cNode = nullptr;
XNODE* iNode = nullptr;
int tok; int tok;
bool cadstarFileCheckDone = false; bool cadstarFileCheckDone = false;
wxString str; wxString str;
@ -2460,7 +2462,10 @@ XNODE* CADSTAR_ARCHIVE_PARSER::LoadArchiveFile( const wxString& aFileName,
if( aProgressReporter && ( currentProgress() - previousReportedProgress ) > 0.01 ) if( aProgressReporter && ( currentProgress() - previousReportedProgress ) > 0.01 )
{ {
if( !aProgressReporter->KeepRefreshing() ) if( !aProgressReporter->KeepRefreshing() )
{
delete rootNode;
THROW_IO_ERROR( _( "File import cancelled by user." ) ); THROW_IO_ERROR( _( "File import cancelled by user." ) );
}
aProgressReporter->SetCurrentProgress( currentProgress() ); aProgressReporter->SetCurrentProgress( currentProgress() );
previousReportedProgress = currentProgress(); previousReportedProgress = currentProgress();
@ -2476,6 +2481,7 @@ XNODE* CADSTAR_ARCHIVE_PARSER::LoadArchiveFile( const wxString& aFileName,
else else
{ {
//too many closing brackets //too many closing brackets
delete rootNode;
THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) ); THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) );
} }
} }
@ -2485,6 +2491,9 @@ XNODE* CADSTAR_ARCHIVE_PARSER::LoadArchiveFile( const wxString& aFileName,
str = wxString( lexer.CurText(), *conv ); str = wxString( lexer.CurText(), *conv );
cNode = new XNODE( wxXML_ELEMENT_NODE, str ); cNode = new XNODE( wxXML_ELEMENT_NODE, str );
if( !rootNode )
rootNode = cNode;
if( iNode ) if( iNode )
{ {
//we will add it as attribute as well as child node //we will add it as attribute as well as child node
@ -2494,7 +2503,10 @@ XNODE* CADSTAR_ARCHIVE_PARSER::LoadArchiveFile( const wxString& aFileName,
else if( !cadstarFileCheckDone ) else if( !cadstarFileCheckDone )
{ {
if( cNode->GetName() != aFileTypeIdentifier ) if( cNode->GetName() != aFileTypeIdentifier )
{
delete rootNode;
THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) ); THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) );
}
cadstarFileCheckDone = true; cadstarFileCheckDone = true;
} }
@ -2510,19 +2522,28 @@ XNODE* CADSTAR_ARCHIVE_PARSER::LoadArchiveFile( const wxString& aFileName,
else else
{ {
//not enough closing brackets //not enough closing brackets
delete rootNode;
THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) ); THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) );
} }
} }
// Not enough closing brackets // Not enough closing brackets
if( iNode != nullptr ) if( iNode != nullptr )
{
delete rootNode;
THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) ); THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) );
}
// Throw if no data was parsed // Throw if no data was parsed
if( cNode ) if( rootNode )
return cNode; {
return rootNode;
}
else else
{
delete rootNode;
THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) ); THROW_IO_ERROR( _( "The selected file is not valid or might be corrupt!" ) );
}
return nullptr; return nullptr;
} }

2
common/plugins/cadstar/cadstar_archive_parser.h
View File

@ -93,6 +93,8 @@ class CADSTAR_ARCHIVE_PARSER
public: public:
CADSTAR_ARCHIVE_PARSER() { m_progressReporter = nullptr; } CADSTAR_ARCHIVE_PARSER() { m_progressReporter = nullptr; }
virtual ~CADSTAR_ARCHIVE_PARSER() {}
typedef wxString LINECODE_ID; typedef wxString LINECODE_ID;
typedef wxString HATCHCODE_ID; typedef wxString HATCHCODE_ID;

9
eeschema/sch_plugins/cadstar/cadstar_sch_archive_parser.cpp
View File

@ -35,7 +35,7 @@ void CADSTAR_SCH_ARCHIVE_PARSER::Parse()
if( m_progressReporter ) if( m_progressReporter )
m_progressReporter->BeginPhase( 0 ); // Read file m_progressReporter->BeginPhase( 0 ); // Read file
XNODE* fileRootNode = LoadArchiveFile( Filename, wxT( "CADSTARSCM" ), m_progressReporter ); m_rootNode = LoadArchiveFile( Filename, wxT( "CADSTARSCM" ), m_progressReporter );
if( m_progressReporter ) if( m_progressReporter )
{ {
@ -44,13 +44,13 @@ void CADSTAR_SCH_ARCHIVE_PARSER::Parse()
std::vector<wxString> subNodeChildrenToCount = { wxT( "LIBRARY" ), wxT( "PARTS" ), std::vector<wxString> subNodeChildrenToCount = { wxT( "LIBRARY" ), wxT( "PARTS" ),
wxT( "SCHEMATIC" ) }; wxT( "SCHEMATIC" ) };
long numOfSteps = GetNumberOfStepsForReporting( fileRootNode, subNodeChildrenToCount ); long numOfSteps = GetNumberOfStepsForReporting( m_rootNode, subNodeChildrenToCount );
m_progressReporter->SetMaxProgress( numOfSteps ); m_progressReporter->SetMaxProgress( numOfSteps );
} }
m_context.CheckPointCallback = [&](){ checkPoint(); }; m_context.CheckPointCallback = [&](){ checkPoint(); };
XNODE* cNode = fileRootNode->GetChildren(); XNODE* cNode = m_rootNode->GetChildren();
if( !cNode ) if( !cNode )
THROW_MISSING_NODE_IO_ERROR( wxT( "HEADER" ), wxT( "CADSTARSCM" ) ); THROW_MISSING_NODE_IO_ERROR( wxT( "HEADER" ), wxT( "CADSTARSCM" ) );
@ -145,7 +145,8 @@ void CADSTAR_SCH_ARCHIVE_PARSER::Parse()
checkPoint(); checkPoint();
} }
delete fileRootNode; delete m_rootNode;
m_rootNode = nullptr;
} }

14
eeschema/sch_plugins/cadstar/cadstar_sch_archive_parser.h
View File

@ -35,11 +35,18 @@
class CADSTAR_SCH_ARCHIVE_PARSER : public CADSTAR_ARCHIVE_PARSER class CADSTAR_SCH_ARCHIVE_PARSER : public CADSTAR_ARCHIVE_PARSER
{ {
public: public:
explicit CADSTAR_SCH_ARCHIVE_PARSER( wxString aFilename ) explicit CADSTAR_SCH_ARCHIVE_PARSER( wxString aFilename ) :
: CADSTAR_ARCHIVE_PARSER(), Filename( aFilename ), Header(), Assignments(), KiCadUnitDivider( 10 ) CADSTAR_ARCHIVE_PARSER(), Filename( aFilename ), Header(), Assignments(),
KiCadUnitDivider( 10 ), m_rootNode( nullptr )
{ {
} }
virtual ~CADSTAR_SCH_ARCHIVE_PARSER()
{
if( m_rootNode )
delete m_rootNode;
}
/** /**
* @brief Parses the file * @brief Parses the file
* @throw IO_ERROR if file could not be opened or there was * @throw IO_ERROR if file could not be opened or there was
@ -454,6 +461,9 @@ public:
int KiCadUnitDivider; ///<Use this value to convert units in this CSA file to KiCad units int KiCadUnitDivider; ///<Use this value to convert units in this CSA file to KiCad units
private:
XNODE* m_rootNode; // Currently parsed root node
}; //CADSTAR_SCH_ARCHIVE_PARSER }; //CADSTAR_SCH_ARCHIVE_PARSER
#endif // CADSTAR_SCH_ARCHIVE_PARSER_H_ #endif // CADSTAR_SCH_ARCHIVE_PARSER_H_

8
pcbnew/plugins/cadstar/cadstar_pcb_archive_parser.cpp
View File

@ -35,7 +35,7 @@ void CADSTAR_PCB_ARCHIVE_PARSER::Parse()
if( m_progressReporter ) if( m_progressReporter )
m_progressReporter->BeginPhase( 0 ); // Read file m_progressReporter->BeginPhase( 0 ); // Read file
XNODE* fileRootNode = LoadArchiveFile( Filename, wxT( "CADSTARPCB" ), m_progressReporter ); m_rootNode = LoadArchiveFile( Filename, wxT( "CADSTARPCB" ), m_progressReporter );
if( m_progressReporter ) if( m_progressReporter )
{ {
@ -44,13 +44,13 @@ void CADSTAR_PCB_ARCHIVE_PARSER::Parse()
std::vector<wxString> subNodeChildrenToCount = { wxT( "LIBRARY" ), wxT( "PARTS" ), std::vector<wxString> subNodeChildrenToCount = { wxT( "LIBRARY" ), wxT( "PARTS" ),
wxT( "LAYOUT" ) }; wxT( "LAYOUT" ) };
long numOfSteps = GetNumberOfStepsForReporting( fileRootNode, subNodeChildrenToCount ); long numOfSteps = GetNumberOfStepsForReporting( m_rootNode, subNodeChildrenToCount );
m_progressReporter->SetMaxProgress( numOfSteps ); m_progressReporter->SetMaxProgress( numOfSteps );
} }
m_context.CheckPointCallback = [&](){ checkPoint(); }; m_context.CheckPointCallback = [&](){ checkPoint(); };
XNODE* cNode = fileRootNode->GetChildren(); XNODE* cNode = m_rootNode->GetChildren();
if( !cNode ) if( !cNode )
THROW_MISSING_NODE_IO_ERROR( wxT( "HEADER" ), wxT( "CADSTARPCB" ) ); THROW_MISSING_NODE_IO_ERROR( wxT( "HEADER" ), wxT( "CADSTARPCB" ) );
@ -123,7 +123,7 @@ void CADSTAR_PCB_ARCHIVE_PARSER::Parse()
checkPoint(); checkPoint();
} }
delete fileRootNode; delete m_rootNode;
} }

15
pcbnew/plugins/cadstar/cadstar_pcb_archive_parser.h
View File

@ -42,12 +42,20 @@
class CADSTAR_PCB_ARCHIVE_PARSER : public CADSTAR_ARCHIVE_PARSER class CADSTAR_PCB_ARCHIVE_PARSER : public CADSTAR_ARCHIVE_PARSER
{ {
public: public:
explicit CADSTAR_PCB_ARCHIVE_PARSER( const wxString& aFilename ) explicit CADSTAR_PCB_ARCHIVE_PARSER( const wxString& aFilename ) :
: Filename( aFilename ), Header(), Assignments(), CADSTAR_ARCHIVE_PARSER() Filename( aFilename ), Header(), Assignments(),
m_rootNode( nullptr ), CADSTAR_ARCHIVE_PARSER()
{ {
KiCadUnitMultiplier = 10; // assume hundredth micron KiCadUnitMultiplier = 10; // assume hundredth micron
} }
virtual ~CADSTAR_PCB_ARCHIVE_PARSER()
{
if( m_rootNode )
delete m_rootNode;
}
/** /**
* @brief Parses the file * @brief Parses the file
* @throw IO_ERROR if file could not be opened or there was * @throw IO_ERROR if file could not be opened or there was
@ -1215,6 +1223,9 @@ public:
int KiCadUnitMultiplier; ///<Use this value to convert units in this CPA file to KiCad units int KiCadUnitMultiplier; ///<Use this value to convert units in this CPA file to KiCad units
private:
XNODE* m_rootNode; // Currently parsed root node
}; //CADSTAR_PCB_ARCHIVE_PARSER }; //CADSTAR_PCB_ARCHIVE_PARSER
#endif // CADSTAR_PCB_ARCHIVE_PARSER_H_ #endif // CADSTAR_PCB_ARCHIVE_PARSER_H_