Sexpr: Do not leak memory on parse exception

Use std::unique_ptr to guard against leaking the new'd pointers
in SEXPR lists if there is an exception during parse.

This was noticable during the Sexpr/ParseExceptions unit test.

Fixes: lp:1830209
* https://bugs.launchpad.net/kicad/+bug/1830209
This commit is contained in:
John Beard 2019-05-23 13:09:38 +01:00
parent 83e08c9277
commit a045642b7e
2 changed files with 25 additions and 17 deletions

View File

@ -19,6 +19,8 @@
#define SEXPR_PARSER_H_ #define SEXPR_PARSER_H_
#include "sexpr/sexpr.h" #include "sexpr/sexpr.h"
#include <memory>
#include <string> #include <string>
#include <vector> #include <vector>
@ -30,12 +32,13 @@ namespace SEXPR
public: public:
PARSER(); PARSER();
~PARSER(); ~PARSER();
SEXPR* Parse( const std::string &aString ); std::unique_ptr<SEXPR> Parse( const std::string& aString );
SEXPR* ParseFromFile( const std::string &aFilename ); std::unique_ptr<SEXPR> ParseFromFile( const std::string& aFilename );
static std::string GetFileContents( const std::string &aFilename ); static std::string GetFileContents( const std::string &aFilename );
private: private:
SEXPR* parseString( const std::string& aString, std::string::const_iterator& it ); std::unique_ptr<SEXPR> parseString(
const std::string& aString, std::string::const_iterator& it );
static const std::string whitespaceCharacters; static const std::string whitespaceCharacters;
int m_lineNumber; int m_lineNumber;
}; };

View File

@ -26,7 +26,9 @@
#include <fstream> #include <fstream>
#include <streambuf> #include <streambuf>
#include <wx/file.h> #include <wx/file.h>
#include <macros.h> #include <macros.h>
#include <make_unique.h>
namespace SEXPR namespace SEXPR
{ {
@ -40,13 +42,13 @@ namespace SEXPR
{ {
} }
SEXPR* PARSER::Parse( const std::string &aString ) std::unique_ptr<SEXPR> PARSER::Parse( const std::string& aString )
{ {
std::string::const_iterator it = aString.begin(); std::string::const_iterator it = aString.begin();
return parseString( aString, it ); return parseString( aString, it );
} }
SEXPR* PARSER::ParseFromFile( const std::string &aFileName ) std::unique_ptr<SEXPR> PARSER::ParseFromFile( const std::string& aFileName )
{ {
std::string str = GetFileContents( aFileName ); std::string str = GetFileContents( aFileName );
@ -76,7 +78,8 @@ namespace SEXPR
return str; return str;
} }
SEXPR* PARSER::parseString( const std::string& aString, std::string::const_iterator& it ) std::unique_ptr<SEXPR> PARSER::parseString(
const std::string& aString, std::string::const_iterator& it )
{ {
for( ; it != aString.end(); ++it ) for( ; it != aString.end(); ++it )
{ {
@ -90,7 +93,8 @@ namespace SEXPR
{ {
std::advance( it, 1 ); std::advance( it, 1 );
SEXPR_LIST* list = new SEXPR_LIST( m_lineNumber ); auto list = std::make_unique<SEXPR_LIST>( m_lineNumber );
while( it != aString.end() && *it != ')' ) while( it != aString.end() && *it != ')' )
{ {
//there may be newlines in between atoms of a list, so detect these here //there may be newlines in between atoms of a list, so detect these here
@ -103,8 +107,8 @@ namespace SEXPR
continue; continue;
} }
SEXPR* item = parseString( aString, it ); std::unique_ptr<SEXPR> item = parseString( aString, it );
list->AddChild( item ); list->AddChild( item.release() );
} }
if( it != aString.end() ) if( it != aString.end() )
@ -131,8 +135,8 @@ namespace SEXPR
if( closingPos != std::string::npos ) if( closingPos != std::string::npos )
{ {
SEXPR_STRING* str = new SEXPR_STRING( auto str = std::make_unique<SEXPR_STRING>(
aString.substr( startPos, closingPos - startPos ),m_lineNumber ); aString.substr( startPos, closingPos - startPos ), m_lineNumber );
std::advance( it, closingPos - startPos + 2 ); std::advance( it, closingPos - startPos + 2 );
return str; return str;
@ -156,17 +160,18 @@ namespace SEXPR
( tmp.size() > 1 && tmp[0] == '-' ( tmp.size() > 1 && tmp[0] == '-'
&& tmp.find_first_not_of( "0123456789.", 1 ) == std::string::npos ) ) && tmp.find_first_not_of( "0123456789.", 1 ) == std::string::npos ) )
{ {
SEXPR* res; std::unique_ptr<SEXPR> res;
if( tmp.find( '.' ) != std::string::npos ) if( tmp.find( '.' ) != std::string::npos )
{ {
res = new SEXPR_DOUBLE( strtod( tmp.c_str(), NULL ), m_lineNumber ); res = std::make_unique<SEXPR_DOUBLE>(
strtod( tmp.c_str(), nullptr ), m_lineNumber );
//floating point type //floating point type
} }
else else
{ {
res = new SEXPR_INTEGER( res = std::make_unique<SEXPR_INTEGER>(
strtoll( tmp.c_str(), NULL, 0 ), m_lineNumber ); strtoll( tmp.c_str(), nullptr, 0 ), m_lineNumber );
} }
std::advance( it, closingPos - startPos ); std::advance( it, closingPos - startPos );
@ -174,7 +179,7 @@ namespace SEXPR
} }
else else
{ {
SEXPR_SYMBOL* str = new SEXPR_SYMBOL( tmp, m_lineNumber ); auto str = std::make_unique<SEXPR_SYMBOL>( tmp, m_lineNumber );
std::advance( it, closingPos - startPos ); std::advance( it, closingPos - startPos );
return str; return str;
@ -187,6 +192,6 @@ namespace SEXPR
} }
} }
return NULL; return nullptr;
} }
} }