kicad-developers team mailing list archive

[Jon Evans <jon@xxxxxxxxxxxxx>]

All set.  Included here for reference

On Sat, Feb 24, 2018 at 1:09 PM, Jeff Young <jeff@xxxxxxxxx> wrote:

> Hi Jon,
>
> Cool.  You want to attach that one to the bug?
>
> Cheers,
> Jeff.
>
>
> On 24 Feb 2018, at 17:55, Jon Evans <jon@xxxxxxxxxxxxx> wrote:
>
> Yes it definitely is specific to some kind of library config issue.  I
> think I have one or more invalid library path, which helps in triggering
> the bug.
> Your patch does not completely fix the issue for me.  FP_CACHE::Load() is
> throwing an exception which is passed through validateCache() but then
> unhandled in doLoadFootprint()
>
> If I change it to:
>
>     try
>     {
>         validateCache( aLibraryPath, checkModified );
>     }
>     catch( const IO_ERROR& ioe )
>     {
>         // do nothing with the error
>     }
>
> then it works.
>
> Thanks,
> -Jon
>
> On Sat, Feb 24, 2018 at 12:45 PM, Jeff Young <jeff@xxxxxxxxx> wrote:
>
>> Hey guys,
>>
>> I’m not sure why I can’t reproduce this (perhaps because I’m still on old
>> libraries?), but could you guys try this one out?
>>
>> Cheers,
>> Jeff.
>>
>>
>>
>> On 24 Feb 2018, at 17:32, Jeff Young <jeff@xxxxxxxxx> wrote:
>>
>> Hi Jon,
>>
>> Thanks for looking into this, but the second one will kill performance.
>> I’ve got a slightly different one baking….
>>
>> Cheers,
>> Jeff.
>>
>>
>> On 24 Feb 2018, at 17:18, Jon Evans <jon@xxxxxxxxxxxxx> wrote:
>>
>> See: https://bugs.launchpad.net/kicad/+bug/1751464
>>
>> The first one is simple; m_Layers wasn't guaranteed to be null so in my
>> recent change I introduced a path to crashing when creating a pcb frame.
>>
>> The second one I am not so sure what is intended, since I haven't really
>> looked at the code in this area very much, but there was a path to call
>> LoadEnumeratedFootprint where m_cache is null.  I added a call to
>> validateCache() to fix this, but I am discarding any exceptions thrown
>> because I'm not sure how we want to handle them (on my install at least, I
>> get some exceptions about bad paths, but if I apply this patch, everything
>> seems to work regardless)
>>
>> -Jon
>> <0001-Make-sure-the-footprint-cache-exists-in-LoadEnumerat.patch>
>> <0001-Ensure-m_Layers-is-null-before-it-is-created.patch>
>> _______________________________________________
>> 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
>>
>>
>>
>>
>
>

From 1145335f3f679d583c49ae7a517171b1e698aeb1 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@xxxxxxxxx>
Date: Sat, 24 Feb 2018 17:41:44 +0000
Subject: [PATCH] Fix segfault when not all libraries loaded.

Fixes: lp:1751464
* https://bugs.launchpad.net/kicad/+bug/1751464
---
 pcbnew/kicad_plugin.cpp | 36 +++++++++++++++++++++++++-----------
 pcbnew/kicad_plugin.h   |  5 ++++-
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp
index e874d5b8a..3275be545 100644
--- a/pcbnew/kicad_plugin.cpp
+++ b/pcbnew/kicad_plugin.cpp
@@ -247,7 +247,7 @@ void FP_CACHE::Save()
 
 
 void FP_CACHE::Load()
-{    
+{
     wxDir dir( m_lib_path.GetPath() );
 
     if( !dir.IsOpened() )
@@ -1944,9 +1944,9 @@ void PCB_IO::init( const PROPERTIES* aProperties )
 }
 
 
-void PCB_IO::validateCache( const wxString& aLibraryPath )
+void PCB_IO::validateCache( const wxString& aLibraryPath, bool checkModified )
 {
-    if( !m_cache || m_cache->IsModified() )
+    if( !m_cache || ( checkModified && m_cache->IsModified() ) )
     {
         // a spectacular episode in memory management:
         delete m_cache;
@@ -1991,14 +1991,24 @@ void PCB_IO::FootprintEnumerate( wxArrayString&    aFootprintNames,
 }
 
 
-MODULE* PCB_IO::LoadEnumeratedFootprint( const wxString& aLibraryPath,
-                                         const wxString& aFootprintName,
-                                         const PROPERTIES* aProperties )
+MODULE* PCB_IO::doLoadFootprint( const wxString& aLibraryPath,
+                                 const wxString& aFootprintName,
+                                 const PROPERTIES* aProperties,
+                                 bool checkModified )
 {
     LOCALE_IO   toggle;     // toggles on, then off, the C locale.
 
     init( aProperties );
 
+    try
+    {
+        validateCache( aLibraryPath, checkModified );
+    }
+    catch( const IO_ERROR& ioe )
+    {
+        // do nothing with the error
+    }
+
     const MODULE_MAP& mods = m_cache->GetModules();
 
     MODULE_CITER it = mods.find( aFootprintName );
@@ -2013,14 +2023,18 @@ MODULE* PCB_IO::LoadEnumeratedFootprint( const wxString& aLibraryPath,
 }
 
 
-MODULE* PCB_IO::FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
-                               const PROPERTIES* aProperties )
+MODULE* PCB_IO::LoadEnumeratedFootprint( const wxString& aLibraryPath,
+                                         const wxString& aFootprintName,
+                                         const PROPERTIES* aProperties )
 {
-    LOCALE_IO   toggle;     // toggles on, then off, the C locale.
+    return doLoadFootprint( aLibraryPath, aFootprintName, aProperties, false );
+}
 
-    validateCache( aLibraryPath );
 
-    return LoadEnumeratedFootprint( aLibraryPath, aFootprintName, aProperties );
+MODULE* PCB_IO::FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
+                               const PROPERTIES* aProperties )
+{
+    return doLoadFootprint( aLibraryPath, aFootprintName, aProperties, true );
 }
 
 
diff --git a/pcbnew/kicad_plugin.h b/pcbnew/kicad_plugin.h
index c95459070..497d4c392 100644
--- a/pcbnew/kicad_plugin.h
+++ b/pcbnew/kicad_plugin.h
@@ -191,7 +191,10 @@ protected:
     NETINFO_MAPPING*    m_mapping;  ///< mapping for net codes, so only not empty net codes
                                     ///< are stored with consecutive integers as net codes
 
-    void validateCache( const wxString& aLibraryPath );
+    void validateCache( const wxString& aLibraryPath, bool checkModified = true );
+
+    MODULE* doLoadFootprint( const wxString& aLibraryPath, const wxString& aFootprintName,
+            const PROPERTIES* aProperties, bool checkModified );
 
     void init( const PROPERTIES* aProperties );
 
-- 
2.14.1