← Back to team overview

kicad-developers team mailing list archive

Re: Segfault when pcbnew starts with an invalid file

 

True, I forgot there is a second patch (attached).

Regards,
Orson

On 08/07/2015 01:42 AM, Wayne Stambaugh wrote:
> I'm getting a segfault with or without Orson's patch on Debian Testing
> so maybe there is something else going on here.
> 
> 
> On 08/05/2015 03:17 PM, Chris Pavlina wrote:
>> Regardless of Dick's future plans wrt Python, I do not see why it would 
>> be desirable to call OnPgmExit inside OnRun instead of inside OnExit. It 
>> seems clear to me that this is the correct location for it. wx 
>> documentation and wx source appear to confirm this, as I indicated 
>> earlier.
>>
>> On Wed, Aug 05, 2015 at 10:39:54AM -0400, Wayne Stambaugh wrote:
>>> On 8/5/2015 10:16 AM, Maciej Sumiński wrote:
>>>> Chris has pointed out that it was already discussed [1] and the change
>>>> was not entirely clear. Apparently it had not been wrapped before
>>>> revision 5834, so maybe OnPgmExit() should be still called for both
>>>> Python and non-Python versions of pcbnew?
>>>
>>> This was done because the problem did not occur when wxPython scripting
>>> was disabled and Dick is planning on add wxPython on top of Kicad rather
>>> than a bolted onto the side like our current implementation.
>>> Compiling OnPgmExit() out of non wxPython builds was at his request.
>>> The original crash had to do with wxPython clean up not being called
>>> properly.  I just tried to duplicate this bug on a 32 bit build on
>>> windows by running 'pcbnew foo` with no scripting enabled, and I cannot
>>> get it to crash.  Are you testing this on Linux?
>>>
>>>>
>>>> Regards,
>>>> Orson
>>>>
>>>> 1.
>>>> https://www.mail-archive.com/kicad-developers@xxxxxxxxxxxxxxxxxxx/msg14014.html
>>>>
>>>> On 08/05/2015 11:22 AM, Maciej Sumiński wrote:
>>>>> Pcbnew without scripting support crashes when run with an invalid file
>>>>> given as a parameter (stack overflow in ~wxSingleInstanceChecker()).
>>>>>
>>>>> There is a fix for that, but wrapped with #ifdef
>>>>> KICAD_SCRIPTING_WXPYTHON .. #endif. I removed the mentioned directives
>>>>> and it seems fine here, but I am not confident enough to commit it
>>>>> immediately. Any thoughts?
>>>>>
>>>>> Regards,
>>>>> Orson
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>> More help   : https://help.launchpad.net/ListHelp
>>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~kicad-developers
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 

diff --git a/common/tool/tool_base.cpp b/common/tool/tool_base.cpp
index 8615c93..f9191d5 100644
--- a/common/tool/tool_base.cpp
+++ b/common/tool/tool_base.cpp
@@ -58,20 +58,9 @@ void TOOL_BASE::attachManager( TOOL_MANAGER* aManager )
 }
 
 
-TOOL_SETTINGS::TOOL_SETTINGS( TOOL_BASE* aTool )
+TOOL_SETTINGS::TOOL_SETTINGS( TOOL_BASE* aTool ) :
+    m_tool( aTool )
 {
-    m_tool = aTool;
-
-    if( !aTool )
-    {
-        m_config = NULL;
-        return;
-    }
-
-    // fixme: make independent of pcbnew (post-stable)
-    PCB_EDIT_FRAME* frame = aTool->getEditFrame<PCB_EDIT_FRAME>();
-
-    m_config = frame->GetSettings();
 }
 
 
@@ -93,3 +82,16 @@ wxString TOOL_SETTINGS::getKeyName( const wxString& aEntryName ) const
     key += aEntryName;
     return key;
 }
+
+
+wxConfigBase* TOOL_SETTINGS::getConfigBase() const
+{
+    if( !m_tool )
+        return NULL;
+
+    // fixme: make independent of pcbnew (post-stable)
+    if( PCB_EDIT_FRAME* frame = m_tool->getEditFrame<PCB_EDIT_FRAME>() )
+        return frame->GetSettings();
+
+    return NULL;
+}
diff --git a/include/tool/tool_settings.h b/include/tool/tool_settings.h
index 393afb0..64d2612 100644
--- a/include/tool/tool_settings.h
+++ b/include/tool/tool_settings.h
@@ -38,34 +38,41 @@ class TOOL_BASE;
 class TOOL_SETTINGS
 {
     public:
-        TOOL_SETTINGS ( TOOL_BASE* aTool = NULL );
-        ~TOOL_SETTINGS ();
+        TOOL_SETTINGS( TOOL_BASE* aTool = NULL );
+        ~TOOL_SETTINGS();
 
-        template <class T>
+        template<class T>
         T Get( const wxString& aName, T aDefaultValue ) const
         {
-            if( !m_config )
+            wxConfigBase* config = getConfigBase();
+
+            if( !config )
                 return aDefaultValue;
 
             T tmp = aDefaultValue;
 
-            m_config->Read( getKeyName( aName ), &tmp );
+            config->Read( getKeyName( aName ), &tmp );
             return tmp;
         }
 
-        template <class T>
+        template<class T>
         void Set( const wxString& aName, const T &aValue )
         {
-            if( !m_config )
+            wxConfigBase* config = getConfigBase();
+
+            if( !config )
                 return;
 
-            m_config->Write( getKeyName( aName ), aValue );
+            config->Write( getKeyName( aName ), aValue );
         }
 
     private:
         wxString getKeyName( const wxString& aEntryName ) const;
 
-        wxConfigBase* m_config;
+        ///> Returns pointer to currently used wxConfigBase. It might be NULL, if there is no
+        ///> TOOL_BASE assigned.
+        wxConfigBase* getConfigBase() const;
+
         TOOL_BASE* m_tool;
 
 };

Attachment: signature.asc
Description: OpenPGP digital signature


Follow ups

References