kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #25309
[PATCH]v3 Fix memory leaks with improper wxBaseConfig* usage (model ownership and ownership-transfer with std::unique_ptr)
Finally I catched up. Attached v3 of the patch, the auiManager part is
removed. Sorry for misinterpreting the comments.
Michael
>From 46886bcccbbd4d99faaec9f374a1a5d1d5413c95 Mon Sep 17 00:00:00 2001
From: decimad <michsteinb@xxxxxxxxx>
Date: Tue, 5 Jul 2016 02:31:04 +0200
Subject: [PATCH] Fix memory leaks with improper wxBaseConfig* usage (model
ownership and ownership-transfer with std::unique_ptr)
---
bitmap2component/bitmap2cmp_gui.cpp | 9 ++++++---
common/bin_mod.cpp | 12 ++++--------
common/common.cpp | 9 +++++----
common/hotkeys_basic.cpp | 6 ++----
common/pgm_base.cpp | 11 +++++++----
include/bin_mod.h | 13 +++++++------
include/common.h | 7 ++++---
include/kiface_i.h | 4 ++--
include/pgm_base.h | 11 +++++++----
include/wxstruct.h | 4 ++--
kicad/pgm_kicad.h | 4 ++--
pcb_calculator/pcb_calculator_frame.cpp | 8 ++++----
12 files changed, 52 insertions(+), 46 deletions(-)
diff --git a/bitmap2component/bitmap2cmp_gui.cpp b/bitmap2component/bitmap2cmp_gui.cpp
index 99ad647..1eb3f53 100644
--- a/bitmap2component/bitmap2cmp_gui.cpp
+++ b/bitmap2component/bitmap2cmp_gui.cpp
@@ -2,7 +2,7 @@
* This program source code file is part of KICAD, a free EDA CAD application.
*
* Copyright (C) 1992-2010 jean-pierre.charras
- * Copyright (C) 1992-2015 Kicad Developers, see change_log.txt for contributors.
+ * Copyright (C) 1992-2016 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
@@ -80,7 +80,8 @@ private:
wxString m_ConvertedFileName;
wxSize m_frameSize;
wxPoint m_framePos;
- wxConfigBase* m_config;
+
+ std::unique_ptr< wxConfigBase > m_config;
public:
BM2CMP_FRAME( KIWAY* aKiway, wxWindow* aParent );
@@ -206,6 +207,7 @@ BM2CMP_FRAME::BM2CMP_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
BM2CMP_FRAME::~BM2CMP_FRAME()
{
+ /// @todo Should the config really not be written if BM2CMP is iconized?
if( !m_config || IsIconized() )
return;
@@ -223,7 +225,8 @@ BM2CMP_FRAME::~BM2CMP_FRAME()
m_config->Write( KEYWORD_LAST_FORMAT, m_radioBoxFormat->GetSelection() );
m_config->Write( KEYWORD_LAST_MODLAYER, m_radio_PCBLayer->GetSelection() );
- delete m_config;
+ m_config->Flush();
+ m_config.reset();
/* This needed for OSX: avoids further OnDraw processing after this
* destructor and before the native window is destroyed
diff --git a/common/bin_mod.cpp b/common/bin_mod.cpp
index 116bc20..55edb35 100644
--- a/common/bin_mod.cpp
+++ b/common/bin_mod.cpp
@@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2014 CERN
- * Copyright (C) 2014 KiCad Developers, see CHANGELOG.TXT for contributors.
+ * Copyright (C) 2014-2016 KiCad Developers, see CHANGELOG.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
@@ -27,8 +27,7 @@
BIN_MOD::BIN_MOD( const char* aName ) :
- m_name( aName ),
- m_config( 0 )
+ m_name( aName )
{
}
@@ -55,11 +54,8 @@ void BIN_MOD::End()
if( m_config )
{
m_history.Save( *m_config );
-
- // Deleting a wxConfigBase writes its contents to disk if changed.
- // Might be NULL if called twice, in which case nothing happens.
- delete m_config;
- m_config = 0;
+ m_config->Flush();
+ m_config.reset();
}
}
diff --git a/common/common.cpp b/common/common.cpp
index 976e419..5f6cac0 100644
--- a/common/common.cpp
+++ b/common/common.cpp
@@ -3,7 +3,7 @@
*
* Copyright (C) 2014-2015 Jean-Pierre Charras, jp.charras at wanadoo.fr
* Copyright (C) 2008-2015 Wayne Stambaugh <stambaughw@xxxxxxxxxxx>
- * Copyright (C) 1992-2015 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2016 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
@@ -300,15 +300,16 @@ double RoundTo0( double x, double precision )
}
-wxConfigBase* GetNewConfig( const wxString& aProgName )
+std::unique_ptr< wxConfigBase > GetNewConfig( const wxString& aProgName )
{
wxConfigBase* cfg = 0;
wxFileName configname;
configname.AssignDir( GetKicadConfigPath() );
configname.SetFullName( aProgName );
- cfg = new wxFileConfig( wxT( "" ), wxT( "" ), configname.GetFullPath() );
- return cfg;
+ return std::unique_ptr< wxConfigBase >(
+ new wxFileConfig( wxT(""), wxT(""), configname.GetFullPath() )
+ );
}
wxString GetKicadLockFilePath()
diff --git a/common/hotkeys_basic.cpp b/common/hotkeys_basic.cpp
index c2f775e..74f0b91 100644
--- a/common/hotkeys_basic.cpp
+++ b/common/hotkeys_basic.cpp
@@ -589,9 +589,8 @@ int EDA_BASE_FRAME::WriteHotkeyConfig( struct EDA_HOTKEY_CONFIG* aDescList,
{
wxFileName fn( GetName() );
fn.SetExt( DEFAULT_HOTKEY_FILENAME_EXT );
- wxConfigBase* config = GetNewConfig( fn.GetFullPath() );
+ std::unique_ptr< wxConfigBase > config = GetNewConfig( fn.GetFullPath() );
config->Write( HOTKEYS_CONFIG_KEY, msg );
- delete config;
}
return 1;
@@ -635,7 +634,7 @@ void ReadHotkeyConfig( const wxString& Appname, struct EDA_HOTKEY_CONFIG* aDescL
wxFileName fn( Appname );
fn.SetExt( DEFAULT_HOTKEY_FILENAME_EXT );
- wxConfigBase* config = GetNewConfig( fn.GetFullPath() );
+ std::unique_ptr< wxConfigBase > config = GetNewConfig( fn.GetFullPath() );
if( !config->HasEntry( HOTKEYS_CONFIG_KEY ) )
{
@@ -645,7 +644,6 @@ void ReadHotkeyConfig( const wxString& Appname, struct EDA_HOTKEY_CONFIG* aDescL
wxString data;
config->Read( HOTKEYS_CONFIG_KEY, &data );
- delete config;
ParseHotkeyConfig( data, aDescList );
}
diff --git a/common/pgm_base.cpp b/common/pgm_base.cpp
index 9368534..0644513 100644
--- a/common/pgm_base.cpp
+++ b/common/pgm_base.cpp
@@ -3,7 +3,7 @@
*
* Copyright (C) 2004-2015 Jean-Pierre Charras, jaen-pierre.charras@xxxxxxxxxxxxxxxxxx
* Copyright (C) 2008-2015 Wayne Stambaugh <stambaughw@xxxxxxxxxxx>
- * Copyright (C) 1992-2015 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2016 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
@@ -283,7 +283,6 @@ PGM_BASE::PGM_BASE()
{
m_pgm_checker = NULL;
m_locale = NULL;
- m_common_settings = NULL;
m_wx_app = NULL;
m_show_env_var_dialog = true;
@@ -304,9 +303,13 @@ void PGM_BASE::destroy()
{
// unlike a normal destructor, this is designed to be called more than once safely:
- delete m_common_settings;
- m_common_settings = 0;
+ if( m_common_settings )
+ {
+ m_common_settings->Flush();
+ m_common_settings.reset();
+ }
+ /// @todo Use smart pointers for these too.
delete m_pgm_checker;
m_pgm_checker = 0;
diff --git a/include/bin_mod.h b/include/bin_mod.h
index cbf360d..dd9f706 100644
--- a/include/bin_mod.h
+++ b/include/bin_mod.h
@@ -1,7 +1,7 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
- * Copyright (C) 2014 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2014-2016 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
@@ -34,6 +34,7 @@
#include <search_stack.h>
+#include <memory>
class wxConfigBase;
/**
@@ -52,13 +53,13 @@ struct BIN_MOD
void Init();
void End();
- const char* m_name; ///< name of this binary module, static C string.
+ const char* m_name; ///< name of this binary module, static C string.
+ std::unique_ptr< wxConfigBase > m_config; ///< maybe from $HOME/.${m_name}
- wxConfigBase* m_config; ///< maybe from $HOME/.${m_name}
- wxFileHistory m_history;
- wxString m_help_file;
+ wxFileHistory m_history;
+ wxString m_help_file;
- SEARCH_STACK m_search;
+ SEARCH_STACK m_search;
};
diff --git a/include/common.h b/include/common.h
index 70e7379..a80c677 100644
--- a/include/common.h
+++ b/include/common.h
@@ -43,6 +43,7 @@
#include <colors.h>
#include <atomic>
+#include <memory>
class wxAboutDialogInfo;
@@ -379,10 +380,10 @@ const wxString PrePendPath( const wxString& aEnvVar, const wxString& aPriorityPa
*
* @param aProgName is the name of the program calling this function - can be obtained by
* calling Pgm().App().GetAppName(). This will be the actual file name of the config file.
- * @return A pointer to a new wxConfigBase derived object is returned. The caller is in charge
- * of deleting it.
+ * @return A std::unique_pointer to a new wxConfigBase derived object is returned. Caller
+ * assumes ownership.
*/
-wxConfigBase* GetNewConfig( const wxString& aProgName );
+std::unique_ptr< wxConfigBase > GetNewConfig( const wxString& aProgName );
/**
* Function GetKicadLockFilePath
diff --git a/include/kiface_i.h b/include/kiface_i.h
index 5cb41b8..153df13 100644
--- a/include/kiface_i.h
+++ b/include/kiface_i.h
@@ -1,7 +1,7 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
- * Copyright (C) 1992-2014 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2016 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
@@ -98,7 +98,7 @@ public:
const wxString Name() { return wxString::FromUTF8( m_bm.m_name ); }
- wxConfigBase* KifaceSettings() const { return m_bm.m_config; }
+ wxConfigBase* KifaceSettings() const { return m_bm.m_config.get(); }
/**
* Function StartFlags
diff --git a/include/pgm_base.h b/include/pgm_base.h
index fae237a..32dd306 100644
--- a/include/pgm_base.h
+++ b/include/pgm_base.h
@@ -3,7 +3,7 @@
*
* Copyright (C) 2004-2015 Jean-Pierre Charras, jaen-pierre.charras@xxxxxxxxxxxxxxxxxx
* Copyright (C) 2008-2015 Wayne Stambaugh <stambaughw@xxxxxxxxxxx>
- * Copyright (C) 1992-2015 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2016 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
@@ -31,12 +31,12 @@
#ifndef PGM_BASE_H_
#define PGM_BASE_H_
+#include <memory>
#include <map>
#include <wx/filename.h>
#include <search_stack.h>
#include <wx/gdicmn.h>
-
class wxConfigBase;
class wxSingleInstanceChecker;
class wxApp;
@@ -106,6 +106,9 @@ class PGM_BASE
{
public:
PGM_BASE();
+
+ // @todo: Add virtual or comment that it will never be destroyed through a base
+ // class reference.
~PGM_BASE();
/**
@@ -127,7 +130,7 @@ public:
//----<Cross Module API>-----------------------------------------------------
- VTBL_ENTRY wxConfigBase* CommonSettings() const { return m_common_settings; }
+ VTBL_ENTRY wxConfigBase* CommonSettings() const { return m_common_settings.get(); }
VTBL_ENTRY void SetEditorName( const wxString& aFileName );
@@ -317,7 +320,7 @@ protected:
/// Configuration settings common to all KiCad program modules,
/// like as in $HOME/.kicad_common
- wxConfigBase* m_common_settings;
+ std::unique_ptr< wxConfigBase > m_common_settings;
/// full path to this program
wxString m_bin_dir;
diff --git a/include/wxstruct.h b/include/wxstruct.h
index a6eb79c..8ae9946 100644
--- a/include/wxstruct.h
+++ b/include/wxstruct.h
@@ -3,7 +3,7 @@
*
* Copyright (C) 2009-2015 Jean-Pierre Charras, jp.charras wanadoo.fr
* Copyright (C) 2011-2015 Wayne Stambaugh <stambaughw@xxxxxxxxxxx>
- * Copyright (C) 1992-2015 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2016 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
@@ -196,7 +196,7 @@ public:
const wxString& aTitle, const wxPoint& aPos, const wxSize& aSize,
long aStyle, const wxString& aFrameName );
- ~EDA_BASE_FRAME();
+ virtual ~EDA_BASE_FRAME();
/**
* Function ProcessEvent
diff --git a/kicad/pgm_kicad.h b/kicad/pgm_kicad.h
index 61990d9..4ca9a03 100644
--- a/kicad/pgm_kicad.h
+++ b/kicad/pgm_kicad.h
@@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2014 SoftPLC Corporation, Dick Hollenbeck <dick@xxxxxxxxxxx>
- * Copyright (C) 2014 KiCad Developers, see CHANGELOG.TXT for contributors.
+ * Copyright (C) 2014-2016 KiCad Developers, see CHANGELOG.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
@@ -55,7 +55,7 @@ public:
wxFileHistory& GetFileHistory() { return m_bm.m_history; }
- wxConfigBase* PgmSettings() { return m_bm.m_config; }
+ wxConfigBase* PgmSettings() { return m_bm.m_config.get(); }
SEARCH_STACK& SysSearch() { return m_bm.m_search; }
diff --git a/pcb_calculator/pcb_calculator_frame.cpp b/pcb_calculator/pcb_calculator_frame.cpp
index 476dee9..dd96e64 100644
--- a/pcb_calculator/pcb_calculator_frame.cpp
+++ b/pcb_calculator/pcb_calculator_frame.cpp
@@ -2,7 +2,7 @@
* This program source code file is part of KICAD, a free EDA CAD application.
*
* Copyright (C) 1992-2015 jean-pierre.charras
- * Copyright (C) 1992-2015 Kicad Developers, see change_log.txt for contributors.
+ * Copyright (C) 1992-2016 Kicad Developers, see change_log.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
@@ -83,15 +83,15 @@ PCB_CALCULATOR_FRAME::PCB_CALCULATOR_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
m_attenuator_list.push_back( new ATTENUATOR_SPLITTER() );
m_currAttenuator = m_attenuator_list[0];
- wxConfigBase* config = GetNewConfig( Pgm().App().GetAppName() );
- LoadSettings( config );
+ std::unique_ptr< wxConfigBase > config = GetNewConfig( Pgm().App().GetAppName() );
+ LoadSettings( config.get() );
ReadDataFile();
TranslineTypeSelection( m_currTransLineType );
m_TranslineSelection->SetSelection( m_currTransLineType );
- TW_Init( config );
+ TW_Init( config.get() );
SetAttenuator( m_AttenuatorsSelection->GetSelection() );
--
2.9.0.windows.1
