← Back to team overview

kicad-developers team mailing list archive

[PATCH]v2 Fix memory leaks with improper wxBaseConfig* usage (model ownership and ownership-transfer with std::unique_ptr)

 

Hello,
I removed the "auto" usage, added @todo's and tried to comply with the coding style. If there's anything noncompliant left I'd appreciate a pointer!

Michael
=== modified file 'bitmap2component/bitmap2cmp_gui.cpp'
--- bitmap2component/bitmap2cmp_gui.cpp	2016-06-08 06:32:01 +0000
+++ bitmap2component/bitmap2cmp_gui.cpp	2016-06-28 21:33:44 +0000
@@ -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 @@
     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()
 {
+    /// @todo Should the config really not be written if BM2CMP is iconized?
     if( !m_config || IsIconized() )
         return;
 
@@ -223,7 +225,8 @@
     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

=== modified file 'common/bin_mod.cpp'
--- common/bin_mod.cpp	2016-01-12 15:35:27 +0000
+++ common/bin_mod.cpp	2016-06-28 21:23:20 +0000
@@ -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 @@
     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();
     }
 }
 

=== modified file 'common/common.cpp'
--- common/common.cpp	2016-05-28 16:46:22 +0000
+++ common/common.cpp	2016-06-28 21:35:29 +0000
@@ -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 @@
 }
 
 
-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()

=== modified file 'common/hotkeys_basic.cpp'
--- common/hotkeys_basic.cpp	2016-06-12 01:22:13 +0000
+++ common/hotkeys_basic.cpp	2016-06-28 21:07:18 +0000
@@ -589,9 +589,8 @@
     {
         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 @@
     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 @@
 
     wxString data;
     config->Read( HOTKEYS_CONFIG_KEY, &data );
-    delete config;
 
     ParseHotkeyConfig( data, aDescList );
 }

=== modified file 'common/pgm_base.cpp'
--- common/pgm_base.cpp	2016-05-22 10:31:08 +0000
+++ common/pgm_base.cpp	2016-06-28 22:17:37 +0000
@@ -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 @@
 {
     m_pgm_checker = NULL;
     m_locale = NULL;
-    m_common_settings = NULL;
 
     m_wx_app = NULL;
     m_show_env_var_dialog = true;
@@ -304,9 +303,12 @@
 {
     // 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;
 

=== modified file 'include/bin_mod.h'
--- include/bin_mod.h	2016-01-12 16:33:33 +0000
+++ include/bin_mod.h	2016-06-28 21:20:15 +0000
@@ -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 @@
     void Init();
     void End();
 
-    const char*         m_name;             ///< name of this binary module, static C string.
-
-    wxConfigBase*       m_config;           ///< maybe from $HOME/.${m_name}
-    wxFileHistory       m_history;
-    wxString            m_help_file;
-
-    SEARCH_STACK        m_search;
+    const char*                     m_name;   ///< name of this binary module, static C string.
+    std::unique_ptr< wxConfigBase > m_config; ///< maybe from $HOME/.${m_name}
+
+    wxFileHistory   m_history;
+    wxString        m_help_file;
+
+    SEARCH_STACK    m_search;
 
 
 };

=== modified file 'include/common.h'
--- include/common.h	2016-05-28 16:46:22 +0000
+++ include/common.h	2016-06-28 21:15:31 +0000
@@ -43,6 +43,7 @@
 #include <colors.h>
 
 #include <atomic>
+#include <memory>
 
 
 class wxAboutDialogInfo;
@@ -379,10 +380,10 @@
  *
  * @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

=== modified file 'include/kiface_i.h'
--- include/kiface_i.h	2016-01-12 16:33:33 +0000
+++ include/kiface_i.h	2016-06-28 21:52:36 +0000
@@ -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 @@
 
     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

=== modified file 'include/pgm_base.h'
--- include/pgm_base.h	2016-05-28 16:57:29 +0000
+++ include/pgm_base.h	2016-06-28 22:02:49 +0000
@@ -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 @@
 {
 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 @@
 
     //----<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 @@
 
     /// 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;

=== modified file 'include/wxstruct.h'
--- include/wxstruct.h	2016-03-11 16:40:24 +0000
+++ include/wxstruct.h	2016-06-28 21:55:13 +0000
@@ -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 @@
         const wxString& aTitle, const wxPoint& aPos, const wxSize& aSize,
         long aStyle, const wxString& aFrameName );
 
-    ~EDA_BASE_FRAME();
+    virtual ~EDA_BASE_FRAME();
 
     /**
      * Function ProcessEvent

=== modified file 'kicad/mainframe.cpp'
--- kicad/mainframe.cpp	2016-06-10 17:47:19 +0000
+++ kicad/mainframe.cpp	2016-06-28 21:56:50 +0000
@@ -3,7 +3,7 @@
  *
  * Copyright (C) 2015 Jean-Pierre Charras, jp.charras at wanadoo.fr
  * Copyright (C) 2013 CERN (www.cern.ch)
- * Copyright (C) 2004-2015 KiCad Developers, see change_log.txt for contributors.
+ * Copyright (C) 2004-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
@@ -114,6 +114,13 @@
 KICAD_MANAGER_FRAME::~KICAD_MANAGER_FRAME()
 {
     m_auimgr.UnInit();
+
+    // wxAuiManager doesn't delete managed windows at least in V 3.1
+    // see wxAuiManager::~wxAuiManager() in framemanager.cpp
+    delete m_mainToolBar;
+    delete m_LeftWin;
+    delete m_Launcher;
+    delete m_MessagesBox;
 }
 
 

=== modified file 'kicad/pgm_kicad.h'
--- kicad/pgm_kicad.h	2016-05-28 16:57:29 +0000
+++ kicad/pgm_kicad.h	2016-06-28 21:57:19 +0000
@@ -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 @@
 
     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; }
 

=== modified file 'pcb_calculator/pcb_calculator_frame.cpp'
--- pcb_calculator/pcb_calculator_frame.cpp	2015-08-09 09:03:25 +0000
+++ pcb_calculator/pcb_calculator_frame.cpp	2016-06-28 22:05:12 +0000
@@ -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 @@
     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() );
 


Follow ups