GitHub plugin: fix potential multiple initialization of libcurl bug.

* Prevent libcurl from inadvertently being initialized twice.
* Dynamically load libcurl only when required.
* Reduce the number of worker threads when loading footprint libraries to
  prevent issues with the GitHub plugin.

common/CMakeLists.txt

@@ -283,7 +283,10 @@ set( COMMON_SRCS
 add_library( common STATIC ${COMMON_SRCS} )
 add_dependencies( common lib-dependencies )
 add_dependencies( common version_header )
-target_link_libraries( common ${Boost_LIBRARIES} ${CURL_LIBRARIES} )
+target_link_libraries( common
+    ${Boost_LIBRARIES}
+#    ${CURL_LIBRARIES}      we dynamically link to this ON DEMAND, not at load time
+    )
 
 
 set( PCB_COMMON_SRCS

common/footprint_info.cpp

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2011 Jean-Pierre Charras, <jp.charras@wanadoo.fr>
- * Copyright (C) 2013 SoftPLC Corporation, Dick Hollenbeck <dick@softplc.com>
+ * Copyright (C) 2013-2016 SoftPLC Corporation, Dick Hollenbeck <dick@softplc.com>
  * Copyright (C) 1992-2013 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
@@ -28,7 +28,12 @@
  */
 
 
-#define USE_WORKER_THREADS      1       // 1:yes, 0:no. use worker thread to load libraries
+/**
+    No. concurrent threads doing "http(s) GET". More than 6 is not significantly
+    faster, less than 6 is likely slower. Main thread is in this count, so if
+    set to 1 then no temp threads are created.
+*/
+#define READER_THREADS      6
 
 /*
  * Functions to read footprint libraries and fill m_footprints by available footprints names
@@ -119,20 +124,13 @@ void FOOTPRINT_INFO::load()
 }
 
 
-#define JOBZ                6       // no. libraries per worker thread.  It takes about
-                                    // a second to load a GITHUB library, so assigning
-                                    // this no. libraries to each thread should give a little
-                                    // over this no. seconds total time if the original delay
-                                    // were caused by latencies alone.
-                                    // (If https://github.com does not mind.)
-
 #define NTOLERABLE_ERRORS   4       // max errors before aborting, although threads
                                     // in progress will still pile on for a bit.  e.g. if 9 threads
                                     // expect 9 greater than this.
 
 void FOOTPRINT_LIST::loader_job( const wxString* aNicknameList, int aJobZ )
 {
-    //DBG(printf( "%s: first:'%s' count:%d\n", __func__, (char*) TO_UTF8( *aNicknameList ), aJobZ );)
+    DBG(printf( "%s: first:'%s' aJobZ:%d\n", __func__, TO_UTF8( *aNicknameList ), aJobZ );)
 
     for( int i=0; i<aJobZ; ++i )
     {
@@ -212,8 +210,6 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a
         // do all of them
         nicknames = aTable->GetLogicalLibs();
 
-#if USE_WORKER_THREADS
-
         // Even though the PLUGIN API implementation is the place for the
         // locale toggling, in order to keep LOCAL_IO::C_count at 1 or greater
         // for the duration of all helper threads, we increment by one here via instantiation.
@@ -229,6 +225,8 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a
 
         MYTHREADS threads;
 
+        unsigned jobz = (nicknames.size() + READER_THREADS - 1) / READER_THREADS;
+
         // Give each thread JOBZ nicknames to process.  The last portion of, or if the entire
         // size() is small, I'll do myself.
         for( unsigned i=0; i<nicknames.size();  )
@@ -240,18 +238,17 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a
                 break;
             }
 
-            int jobz = JOBZ;
+            if( i + jobz >= nicknames.size() )  // on the last iteration of this for(;;)
-
-            if( i + jobz >= nicknames.size() )
             {
                 jobz = nicknames.size() - i;
 
-                // Only a little bit to do, I'll do it myself, on current thread.
+                // Only a little bit to do, I'll do it myself on current thread.
+                // I am part of the READER_THREADS count.
                 loader_job( &nicknames[i], jobz );
             }
             else
             {
-                // Delegate the job to a worker thread created here.
+                // Delegate the job to a temporary thread created here.
                 threads.push_back( new boost::thread( &FOOTPRINT_LIST::loader_job,
                         this, &nicknames[i], jobz ) );
             }
@@ -266,9 +263,6 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a
         {
             threads[i].join();
         }
-#else
-        loader_job( &nicknames[0], nicknames.size() );
-#endif
 
         m_list.sort();
     }

common/kicad_curl/kicad_curl.cpp

@@ -22,38 +22,175 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
  */
 
-#include <kicad_curl/kicad_curl.h>
+#include <wx/log.h>
+#include <wx/dynlib.h>
 
-bool KICAD_CURL::Init()
+#include <macros.h>
+#include <kicad_curl/kicad_curl.h>
+#include <ki_mutex.h>       // MUTEX and MUTLOCK
+#include <richio.h>
+
+// These are even more private than class members, and since there is only
+// one instance of KICAD_CURL ever, these statics are hidden here to simplify the
+// client (API) header file.
+static volatile bool s_initialized;
+
+static MUTEX s_lock;
+
+
+void        (CURL_EXTERN * KICAD_CURL::easy_cleanup)    ( CURL* curl );
+CURL*       (CURL_EXTERN * KICAD_CURL::easy_init)       ( void );
+CURLcode    (CURL_EXTERN * KICAD_CURL::easy_perform)    ( CURL* curl );
+CURLcode    (CURL_EXTERN * KICAD_CURL::easy_setopt)     ( CURL* curl, CURLoption option, ... );
+const char* (CURL_EXTERN * KICAD_CURL::easy_strerror)   ( CURLcode );
+CURLcode    (CURL_EXTERN * KICAD_CURL::global_init)     ( long flags );
+void        (CURL_EXTERN * KICAD_CURL::global_cleanup)  ( void );
+curl_slist* (CURL_EXTERN * KICAD_CURL::slist_append)    ( curl_slist*, const char* );
+void        (CURL_EXTERN * KICAD_CURL::slist_free_all)  ( curl_slist* );
+char*       (CURL_EXTERN * KICAD_CURL::version)         ( void );
+curl_version_info_data* (CURL_EXTERN * KICAD_CURL::version_info) (CURLversion);
+
+
+struct DYN_LOOKUP
 {
-    if ( curl_global_init( CURL_GLOBAL_ALL ) != CURLE_OK )
+    const char* name;
+    void**      address;
+};
+
+// May need to modify "name" for each platform according to how libcurl is built on
+// that platform and the spelling or partial mangling of C function names.  On linux
+// there is no mangling.
+#define DYN_PAIR( basename )    { "curl_" #basename, (void**) &KICAD_CURL::basename }
+
+
+const DYN_LOOKUP KICAD_CURL::dyn_funcs[] = {
+    DYN_PAIR( easy_cleanup ),
+    DYN_PAIR( easy_init ),
+    DYN_PAIR( easy_perform ),
+    DYN_PAIR( easy_setopt ),
+    DYN_PAIR( easy_strerror ),
+    DYN_PAIR( global_init ),
+    DYN_PAIR( global_cleanup ),
+    DYN_PAIR( slist_append ),
+    DYN_PAIR( slist_free_all ),
+    DYN_PAIR( version ),
+    DYN_PAIR( version_info ),
+};
+
+
+void KICAD_CURL::Init()
 {
-        return false;
+    // We test s_initialized twice in an effort to avoid
+    // unnecessarily locking s_lock.  This understands that the common case
+    // will not need to lock.
+    if( !s_initialized )
+    {
+        MUTLOCK lock( s_lock );
+
+        if( !s_initialized )
+        {
+            // dynamically load the library.
+            wxDynamicLibrary dso;
+            wxString canonicalName = dso.CanonicalizeName( wxT( "curl" ) );
+
+            // This is an ugly hack for MinGW builds.  We should probably use something
+            // like objdump to get the actual library file name from the link file.
+#if defined( __MINGW32__ )
+            canonicalName = dso.CanonicalizeName( wxT( "curl-4" ) );
+            canonicalName = wxT( "lib" ) + canonicalName;
+#endif
+
+            if( !dso.Load( canonicalName, wxDL_NOW | wxDL_GLOBAL ) )
+            {
+                // Failure: error reporting UI was done via wxLogSysError().
+                std::string msg = StrPrintf( "%s not wxDynamicLibrary::Load()ed",
+                                             canonicalName );
+                THROW_IO_ERROR( msg );
             }
-    else
+
+            // get addresses.
+
+            for( unsigned i=0; i < DIM(dyn_funcs); ++i )
             {
-        m_initialized = true;
+                *dyn_funcs[i].address = dso.GetSymbol( dyn_funcs[i].name );
-        return true;
+
+                if( *dyn_funcs[i].address == NULL )
+                {
+                    // Failure: error reporting UI was done via wxLogSysError().
+                    // No further reporting required here.
+
+                    std::string msg = StrPrintf( "%s has no function %s",
+                            static_cast<const char*>( canonicalName ),
+                            dyn_funcs[i].name
+                            );
+
+                    THROW_IO_ERROR( msg );
+                }
+            }
+
+            if( KICAD_CURL::global_init( CURL_GLOBAL_ALL ) != CURLE_OK )
+            {
+                THROW_IO_ERROR( "curl_global_init() failed." );
+            }
+
+            wxLogDebug( "Using %s", GetVersion() );
+
+            // Tell dso's wxDynamicLibrary destructor not to Unload() the program image,
+            // since everything is fine before this.  In those cases where THROW_IO_ERROR
+            // is called, dso is destroyed and the DSO/DLL is unloaded before returning in
+            // those error cases.
+            (void) dso.Detach();
+
+            s_initialized = true;
+        }
     }
 }
 
 
 void KICAD_CURL::Cleanup()
 {
-    if( m_initialized )
+    /*
-        curl_global_cleanup();
+
+    Calling MUTLOCK() from a static destructor will typically be bad, since the
+    s_lock may already have been statically destroyed itself leading to a boost
+    exception. (Remember C++ does not provide certain sequencing of static
+    destructor invocation.)
+
+    To prevent this we test s_initialized twice, which ensures that the MUTLOCK
+    is only instantiated on the first call, which should be from
+    PGM_BASE::destroy() which is first called earlier than static destruction.
+    Then when called again from the actual PGM_BASE::~PGM_BASE() function,
+    MUTLOCK will not be instantiated because s_initialized will be false.
+
+    */
+
+    if( s_initialized )
+    {
+        MUTLOCK lock( s_lock );
+
+        if( s_initialized )
+        {
+
+            KICAD_CURL::global_cleanup();
+
+            // dyn_funcs are not good for anything now, assuming process is ending soon here.
+            for( unsigned i=0; i < DIM(dyn_funcs);  ++i )
+            {
+                *dyn_funcs[i].address = 0;
             }
 
-
+            s_initialized = false;
-std::string KICAD_CURL::GetVersion()
+        }
-{
+    }
-    return std::string( curl_version() );
 }
 
 
 std::string KICAD_CURL::GetSimpleVersion()
 {
-    curl_version_info_data *info = curl_version_info(CURLVERSION_NOW);
+    if( !s_initialized )
+        Init();
+
+    curl_version_info_data *info = KICAD_CURL::version_info( CURLVERSION_NOW );
 
     std::string res;
 
@@ -63,6 +200,7 @@ std::string KICAD_CURL::GetSimpleVersion()
     }
 
     res += " (";
+
     if( info->features & CURL_VERSION_SSL )
     {
         res += "with SSL - ";
@@ -76,5 +214,3 @@ std::string KICAD_CURL::GetSimpleVersion()
 
     return res;
 }
-
-bool KICAD_CURL::m_initialized = false;

common/kicad_curl/kicad_curl_easy.cpp

@@ -30,134 +30,70 @@
 #include <sstream>
 #include <richio.h>
 
-static size_t write_callback (void *contents, size_t size, size_t nmemb, void *userp);
 
-
+static size_t write_callback( void* contents, size_t size, size_t nmemb, void* userp )
-KICAD_CURL_EASY::KICAD_CURL_EASY()
-    : m_headers( NULL )
 {
-    m_CURL = curl_easy_init();
+    size_t realsize = size * nmemb;
 
-    if( m_CURL == NULL )
+    std::string* p = (std::string*) userp;
+
+    p->append( (const char*) contents, realsize );
+
+    return realsize;
+}
+
+
+KICAD_CURL_EASY::KICAD_CURL_EASY() :
+    m_headers( NULL )
+{
+    // Call KICAD_CURL::Init() from in here everytime, but only the first time
+    // will incur any overhead.  This strategy ensures that libcurl is never loaded
+    // unless it is needed.
+
+    KICAD_CURL::Init();
+
+    // Do not catch exception from KICAD_CURL::Init() at this level.
+    // Instantiation of this instance will fail if Init() throws, thus ensuring
+    // that this instance cannot be subsequently used.
+    // Caller needs a try catch around KICAD_CURL_EASY instantiation.
+
+    m_CURL = KICAD_CURL::easy_init();
+
+    if( !m_CURL )
     {
         THROW_IO_ERROR( "Unable to initialize CURL session" );
     }
 
-    m_Buffer.Payload = (char*)malloc( 1 );
+    KICAD_CURL::easy_setopt( m_CURL, CURLOPT_WRITEFUNCTION, write_callback );
-    m_Buffer.Size = 0;
+    KICAD_CURL::easy_setopt( m_CURL, CURLOPT_WRITEDATA, (void*) &m_buffer );
-
-    curl_easy_setopt( m_CURL, CURLOPT_WRITEFUNCTION, write_callback );
-    curl_easy_setopt( m_CURL, CURLOPT_WRITEDATA, (void *)&m_Buffer );
 }
 
 
 KICAD_CURL_EASY::~KICAD_CURL_EASY()
 {
-    free(m_Buffer.Payload);
+    if( m_headers )
-    curl_easy_cleanup(m_CURL);
+        KICAD_CURL::slist_free_all( m_headers );
-}
 
-
+    KICAD_CURL::easy_cleanup( m_CURL );
-bool KICAD_CURL_EASY::SetURL( const std::string& aURL )
-{
-    if( SetOption<const char *>( CURLOPT_URL, aURL.c_str() ) == CURLE_OK )
-    {
-        return true;
-    }
-    return false;
-}
-
-
-bool KICAD_CURL_EASY::SetUserAgent( const std::string& aAgent )
-{
-    if( SetOption<const char *>( CURLOPT_USERAGENT, aAgent.c_str() ) == CURLE_OK )
-    {
-        return true;
-    }
-    return false;
-}
-
-
-bool KICAD_CURL_EASY::SetFollowRedirects( bool aFollow )
-{
-    if( SetOption<long>( CURLOPT_FOLLOWLOCATION , (aFollow ? 1 : 0) ) == CURLE_OK )
-    {
-        return true;
-    }
-    return false;
-}
-
-
-void KICAD_CURL_EASY::SetHeader( const std::string& aName, const std::string& aValue )
-{
-    std::string header = aName + ':' + aValue;
-    m_headers = curl_slist_append( m_headers, header.c_str() );
-}
-
-
-std::string KICAD_CURL_EASY::GetErrorText(CURLcode code)
-{
-    return curl_easy_strerror(code);
-}
-
-
-static size_t write_callback( void *contents, size_t size, size_t nmemb, void *userp )
-{
-    /* calculate buffer size */
-    size_t realsize = size * nmemb;
-
-    /* cast pointer to fetch struct */
-    struct KICAD_EASY_CURL_BUFFER *p = ( struct KICAD_EASY_CURL_BUFFER * ) userp;
-
-    /* expand buffer */
-    p->Payload = (char *) realloc( p->Payload, p->Size + realsize + 1 );
-
-    /* check buffer */
-    if ( p->Payload == NULL )
-    {
-        wxLogError( wxT( "Failed to expand buffer in curl_callback" ) );
-
-        /* free buffer */
-        free( p->Payload );
-
-        return -1;
-    }
-
-    /* copy contents to buffer */
-    memcpy( &(p->Payload[p->Size]), contents, realsize );
-
-    /* set new buffer size */
-    p->Size += realsize;
-
-    /* ensure null termination */
-    p->Payload[p->Size] = 0;
-
-    /* return size */
-    return realsize;
 }
 
 
 void KICAD_CURL_EASY::Perform()
 {
-    if( m_headers != NULL )
+    if( m_headers )
     {
-        curl_easy_setopt( m_CURL, CURLOPT_HTTPHEADER, m_headers );
+        KICAD_CURL::easy_setopt( m_CURL, CURLOPT_HTTPHEADER, m_headers );
     }
 
-    if( m_Buffer.Size > 0 )
+    // bonus: retain worst case memory allocation, should re-use occur
-    {
+    m_buffer.clear();
-        free( m_Buffer.Payload );
+
-        m_Buffer.Payload = (char*)malloc( 1 );
+    CURLcode res = KICAD_CURL::easy_perform( m_CURL );
-        m_Buffer.Size = 0;
-    }
 
-    CURLcode res = curl_easy_perform( m_CURL );
     if( res != CURLE_OK )
     {
-        wxString msg = wxString::Format(
+        std::string msg = StrPrintf( "curl_easy_perform()=%d: %s",
-            _( "CURL Request Failed: %s" ),
+                            res, GetErrorText( res ).c_str() );
-            GetErrorText( res ) );
-
         THROW_IO_ERROR( msg );
     }
 }

common/pgm_base.cpp

@@ -283,7 +283,6 @@ PGM_BASE::PGM_BASE()
 PGM_BASE::~PGM_BASE()
 {
     destroy();
-    KICAD_CURL::Cleanup();
 }
 
 
@@ -291,6 +290,8 @@ void PGM_BASE::destroy()
 {
     // unlike a normal destructor, this is designed to be called more than once safely:
 
+    KICAD_CURL::Cleanup();
+
     delete m_common_settings;
     m_common_settings = 0;
 
@@ -495,13 +496,6 @@ bool PGM_BASE::initPgm()
     wxSystemOptions::SetOption( wxOSX_FILEDIALOG_ALWAYS_SHOW_TYPES, 1 );
 #endif
 
-    // Initialize CURL
-    wxLogDebug( wxT( "Using %s" ), KICAD_CURL::GetVersion() );
-    if( !KICAD_CURL::Init() )
-    {
-        wxLogDebug( wxT( "Error initializing libcurl" ) );
-    }
-
     return true;
 }
 

include/kicad_curl/kicad_curl.h

@@ -44,12 +44,27 @@
 #include <curl/curl.h>
 #include <string>
 
+// CURL_EXTERN expands to dllimport on MinGW which causes gcc warnings.  This really should
+// expand to nothing on MinGW.
+#if defined( __MINGW32__)
+#  if defined( CURL_EXTERN )
+#    undef CURL_EXTERN
+#    define CURL_EXTERN
+#  endif
+#endif
+
+
+struct DYN_LOOKUP;
+
+
 /**
  * Class KICAD_CURL
  * simple wrapper class to call curl_global_init and curl_global_cleanup for KiCad.
  */
 class KICAD_CURL
 {
+    friend class KICAD_CURL_EASY;
+
 public:
     /**
      * Function Init
@@ -57,8 +72,9 @@ public:
      * and before any curl functions that perform requests.
      *
      * @return bool - True if successful, false if CURL returned an error
+     * @throw IO_ERROR on failure, hopefully with helpful text in it.
      */
-    static bool Init();
+    static void Init();
 
     /**
      * Function Cleanup
@@ -71,9 +87,14 @@ public:
      * Function GetVersion
      * wrapper for curl_version(). Reports back a short string of loaded libraries.
      *
-     * @return std::string - String reported by libcurl
+     * @return const char* - String reported by libcurl and owned by it.
+     * @throw IO_ERROR on failure, hopefully with helpful text in it.
      */
-    static std::string GetVersion();
+    static const char* GetVersion()
+    {
+        return KICAD_CURL::version();
+    }
+
 
     /**
      * Function GetSimpleVersion
@@ -83,7 +104,25 @@ public:
      */
     static std::string GetSimpleVersion();
 private:
-    static bool m_initialized;
+
+    // Alphabetically:
+    // dynamically looked up libcurl function pointers whose prototypes were
+    // taken from the system's libcurl headers.
+
+    static void         (CURL_EXTERN * easy_cleanup)    ( CURL* curl );
+    static CURL*        (CURL_EXTERN * easy_init)       ( void );
+    static CURLcode     (CURL_EXTERN * easy_perform)    ( CURL* curl );
+    static CURLcode     (CURL_EXTERN * easy_setopt)     ( CURL* curl, CURLoption option, ... );
+    static const char*  (CURL_EXTERN * easy_strerror)   ( CURLcode );
+    static CURLcode     (CURL_EXTERN * global_init)     ( long flags );
+    static void         (CURL_EXTERN * global_cleanup)  ( void );
+    static curl_slist*  (CURL_EXTERN * slist_append)    ( curl_slist*, const char* );
+    static void         (CURL_EXTERN * slist_free_all)  ( curl_slist* );
+    static char*        (CURL_EXTERN * version)         ( void );
+    static curl_version_info_data* (CURL_EXTERN * version_info) (CURLversion);
+
+    /// A tuple of ASCII function names and pointers to pointers to functions
+    static const DYN_LOOKUP dyn_funcs[];
 };
 
 #endif // KICAD_CURL_H_

include/kicad_curl/kicad_curl_easy.h

@@ -42,19 +42,9 @@
 #endif
 
 
-#include <curl/curl.h>
 #include <string>
-
+#include <curl/curl.h>
-/**
+#include <kicad_curl/kicad_curl.h>
- * Struct KICAD_EASY_CURL_BUFFER
- * is a struct used for storing the libcurl received data in its callbacks.
- * Do not use directly, KICAD_CURL_EASY uses it.
- */
-struct KICAD_EASY_CURL_BUFFER
-{
-    char* Payload;
-    size_t Size;
-};
 
 
 /**
@@ -67,8 +57,9 @@ struct KICAD_EASY_CURL_BUFFER
  * Here is a small example usage:
  * @code
  *   KICAD_CURL_EASY curl;
+ *
  *   curl.SetURL( "http://github.com" );
- *   curl.SetUserAgent("KiCad-EDA");
+ *   curl.SetUserAgent( <http-client-indentifier> );
  *   curl.SetHeader( "Accept", "application/json" );
  *   curl.Perform();
  * @endcode
@@ -95,7 +86,11 @@ public:
      * @param aName is the left hand side of the header, i.e. Accept without the colon
      * @param aValue is the right hand side of the header, i.e. application/json
      */
-    void SetHeader( const std::string& aName, const std::string& aValue );
+    void SetHeader( const std::string& aName, const std::string& aValue )
+    {
+        std::string header = aName + ':' + aValue;
+        m_headers = KICAD_CURL::slist_append( m_headers, header.c_str() );
+    }
 
     /**
      * Function SetUserAgent
@@ -104,7 +99,14 @@ public:
      * @param aAgent is the string to set for the user agent
      * @return bool - True if successful, false if not
      */
-    bool SetUserAgent( const std::string& aAgent );
+    bool SetUserAgent( const std::string& aAgent )
+    {
+        if( SetOption<const char*>( CURLOPT_USERAGENT, aAgent.c_str() ) == CURLE_OK )
+        {
+            return true;
+        }
+        return false;
+    }
 
     /**
      * Function SetURL
@@ -113,7 +115,14 @@ public:
      * @param aURL is the URL
      * @return bool - True if successful, false if not
      */
-    bool SetURL( const std::string& aURL );
+    bool SetURL( const std::string& aURL )
+    {
+        if( SetOption<const char *>( CURLOPT_URL, aURL.c_str() ) == CURLE_OK )
+        {
+            return true;
+        }
+        return false;
+    }
 
     /**
      * Function SetFollowRedirects
@@ -123,16 +132,26 @@ public:
      * @param aFollow is a boolean where true will enable following redirects
      * @return bool - True if successful, false if not
      */
-    bool SetFollowRedirects( bool aFollow );
+    bool SetFollowRedirects( bool aFollow )
+    {
+        if( SetOption<long>( CURLOPT_FOLLOWLOCATION , (aFollow ? 1 : 0) ) == CURLE_OK )
+        {
+            return true;
+        }
+        return false;
+    }
 
     /**
      * Function GetErrorText
      * fetches CURL's "friendly" error string for a given error code
      *
      * @param aCode is CURL error code
-     * @return std::string - the corresponding error string for the given code
+     * @return const std::string - the corresponding error string for the given code
      */
-    std::string GetErrorText( CURLcode aCode );
+    const std::string GetErrorText( CURLcode aCode )
+    {
+        return KICAD_CURL::easy_strerror( aCode );
+    }
 
     /**
      * Function SetOption
@@ -144,23 +163,22 @@ public:
      */
     template <typename T> CURLcode SetOption( CURLoption aOption, T aArg )
     {
-        return curl_easy_setopt( m_CURL, aOption, aArg ); 
+        return KICAD_CURL::easy_setopt( m_CURL, aOption, aArg );
     }
 
     /**
      * Function GetBuffer
-     * returns a const pointer to the data buffer
+     * returns a const reference to the recevied data buffer
-     *
-     * @return KICAD_EASY_CURL_BUFFER* - pointer to buffer
      */
-    const KICAD_EASY_CURL_BUFFER* GetBuffer()
+    const std::string& GetBuffer()
     {
-        return &m_Buffer;
+        return m_buffer;
     }
+
 private:
     CURL*           m_CURL;
-    struct curl_slist *m_headers;
+    curl_slist*     m_headers;
-    struct KICAD_EASY_CURL_BUFFER m_Buffer;
+    std::string     m_buffer;
 };
 
 #endif // KICAD_CURL_EASY_H_

pcbnew/CMakeLists.txt

@@ -426,11 +426,11 @@ if( KICAD_SCRIPTING_MODULES )
         pcad2kicadpcb
         lib_dxf
         idf3
-        ${GITHUB_PLUGIN_LIBRARIES}
         polygon
         bitmaps
         gal
         ${wxWidgets_LIBRARIES}
+        ${GITHUB_PLUGIN_LIBRARIES}
         ${GDI_PLUS_LIBRARIES}
         ${PYTHON_LIBRARIES}
         ${PCBNEW_EXTRA_LIBS}
@@ -594,8 +594,8 @@ target_link_libraries( pcbnew_kiface
     gal
     lib_dxf
     idf3
-    ${GITHUB_PLUGIN_LIBRARIES}
     ${wxWidgets_LIBRARIES}
+    ${GITHUB_PLUGIN_LIBRARIES}
     ${GDI_PLUS_LIBRARIES}
     ${PYTHON_LIBRARIES}
     ${Boost_LIBRARIES}      # must follow GITHUB

pcbnew/github/github_getliblist.cpp

@@ -41,7 +41,7 @@
  *  JP Charras.
  */
 
-#include <kicad_curl/kicad_curl_easy.h> /* Include before any wx file */
+#include <kicad_curl/kicad_curl_easy.h>     // Include before any wx file
 #include <wx/uri.h>
 
 #include <github_getliblist.h>
@@ -62,6 +62,7 @@ bool GITHUB_GETLIBLIST::Get3DshapesLibsList( wxArrayString* aList,
                         bool (*aFilter)( const wxString& aData ) )
 {
     std::string fullURLCommand;
+
     strcpy( m_option_string, "text/html" );
 
     wxString repoURL = m_repoURL;
@@ -95,6 +96,7 @@ bool GITHUB_GETLIBLIST::GetFootprintLibraryList( wxArrayString& aList )
     std::string fullURLCommand;
     int page = 1;
     int itemCountMax = 99;              // Do not use a valu > 100, it does not work
+
     strcpy( m_option_string, "application/json" );
 
     // Github max items returned is 100 per page
@@ -213,15 +215,14 @@ bool GITHUB_GETLIBLIST::remoteGetJSON( const std::string& aFullURLCommand, wxStr
     wxLogDebug( wxT( "Attempting to download: " ) + aFullURLCommand );
 
     kcurl.SetURL( aFullURLCommand );
-    kcurl.SetUserAgent("KiCad-EDA");
+    kcurl.SetUserAgent( "http://kicad-pcb.org" );
     kcurl.SetHeader( "Accept", m_option_string );
     kcurl.SetFollowRedirects( true );
 
     try
     {
         kcurl.Perform();
-        m_image.reserve( kcurl.GetBuffer()->Size );
+        m_image = kcurl.GetBuffer();
-        m_image.assign( kcurl.GetBuffer()->Payload, kcurl.GetBuffer()->Size );
         return true;
     }
     catch( const IO_ERROR& ioe )

pcbnew/github/github_plugin.cpp

@@ -61,10 +61,9 @@ Access-Control-Expose-Headers: ETag, Link, X-RateLimit-Limit, X-RateLimit-Remain
 Access-Control-Allow-Origin: *
 X-GitHub-Request-Id: 411087C2:659E:50FD6E6:52E67F66
 Vary: Accept-Encoding
-
 */
-#include <kicad_curl/kicad_curl_easy.h> /* Include before any wx file */
 
+#include <kicad_curl/kicad_curl_easy.h>     // Include before any wx file
 #include <sstream>
 #include <boost/ptr_container/ptr_map.hpp>
 #include <set>
@@ -122,7 +121,7 @@ GITHUB_PLUGIN::~GITHUB_PLUGIN()
 
 const wxString GITHUB_PLUGIN::PluginName() const
 {
-    return wxT( "Github" );
+    return "Github";
 }
 
 
@@ -391,7 +390,7 @@ void GITHUB_PLUGIN::cacheLib( const wxString& aLibraryPath, const PROPERTIES* aP
 
                 if( !wx_pretty_fn.IsOk() ||
                     !wx_pretty_fn.IsDirWritable() ||
-                    wx_pretty_fn.GetExt() != wxT( "pretty" )
+                    wx_pretty_fn.GetExt() != "pretty"
                   )
                 {
                     wxString msg = wxString::Format(
@@ -408,7 +407,7 @@ void GITHUB_PLUGIN::cacheLib( const wxString& aLibraryPath, const PROPERTIES* aP
         }
 
         // operator==( wxString, wxChar* ) does not exist, construct wxString once here.
-        const wxString    kicad_mod( wxT( "kicad_mod" ) );
+        const wxString    kicad_mod( "kicad_mod" );
 
         //D(printf("%s: this:%p  m_lib_path:'%s'  aLibraryPath:'%s'\n", __func__, this, TO_UTF8( m_lib_path), TO_UTF8(aLibraryPath) );)
         m_gh_cache = new GH_CACHE();
@@ -443,7 +442,7 @@ void GITHUB_PLUGIN::cacheLib( const wxString& aLibraryPath, const PROPERTIES* aP
 }
 
 
-bool GITHUB_PLUGIN::repoURL_zipURL( const wxString& aRepoURL, std::string& aZipURL )
+bool GITHUB_PLUGIN::repoURL_zipURL( const wxString& aRepoURL, std::string* aZipURL )
 {
     // e.g. "https://github.com/liftoff-sr/pretty_footprints"
     //D(printf("aRepoURL:%s\n", TO_UTF8( aRepoURL ) );)
@@ -509,7 +508,7 @@ bool GITHUB_PLUGIN::repoURL_zipURL( const wxString& aRepoURL, std::string& aZipU
             // this code path with the needs of one particular inflexible server.
         }
 
-        aZipURL = zip_url.utf8_str();
+        *aZipURL = zip_url.utf8_str();
         return true;
     }
     return false;
@@ -520,7 +519,7 @@ void GITHUB_PLUGIN::remoteGetZip( const wxString& aRepoURL ) throw( IO_ERROR )
 {
     std::string  zip_url;
 
-    if( !repoURL_zipURL( aRepoURL, zip_url ) )
+    if( !repoURL_zipURL( aRepoURL, &zip_url ) )
     {
         wxString msg = wxString::Format( _( "Unable to parse URL:\n'%s'" ), GetChars( aRepoURL ) );
         THROW_IO_ERROR( msg );
@@ -528,27 +527,30 @@ void GITHUB_PLUGIN::remoteGetZip( const wxString& aRepoURL ) throw( IO_ERROR )
 
     wxLogDebug( wxT( "Attempting to download: " ) + zip_url );
 
-    KICAD_CURL_EASY kcurl;
+    KICAD_CURL_EASY kcurl;      // this can THROW_IO_ERROR
 
     kcurl.SetURL( zip_url.c_str() );
-    kcurl.SetUserAgent("KiCad-EDA");
+    kcurl.SetUserAgent( "http://kicad-pcb.org" );
     kcurl.SetHeader( "Accept", "application/zip" );
     kcurl.SetFollowRedirects( true );
 
     try
     {
         kcurl.Perform();
-        m_zip_image.reserve( kcurl.GetBuffer()->Size );
+        m_zip_image = kcurl.GetBuffer();
-        m_zip_image.assign( kcurl.GetBuffer()->Payload, kcurl.GetBuffer()->Size );
     }
     catch( const IO_ERROR& ioe )
     {
+        // https "GET" has faild, report this to API caller.
+        static const char errorcmd[] = "http GET command failed";  // Do not translate this message
+
         UTF8 fmt( _( "%s\nCannot get/download Zip archive: '%s'\nfor library path: '%s'.\nReason: '%s'" ) );
 
         std::string msg = StrPrintf( fmt.c_str(),
                                      zip_url.c_str(),
                                      TO_UTF8( aRepoURL ),
-                                     TO_UTF8( ioe.errorText ) );
+                                     TO_UTF8( ioe.errorText )
+                                     );
 
         THROW_IO_ERROR( msg );
     }
@@ -575,7 +577,7 @@ int main( int argc, char** argv )
     try
     {
         wxArrayString fps = gh.FootprintEnumerate(
-                wxT( "https://github.com/liftoff-sr/pretty_footprints" ),
+                "https://github.com/liftoff-sr/pretty_footprints",
                 NULL
                 );
 

pcbnew/github/github_plugin.h

@@ -210,7 +210,7 @@ protected:
      * @param  aZipURL is where to put the zip file URL.
      * @return bool - true if @a aRepoULR was parseable, else false
      */
-    static bool repoURL_zipURL( const wxString& aRepoURL, std::string& aZipURL );
+    static bool repoURL_zipURL( const wxString& aRepoURL, std::string* aZipURL );
 
     /**
      * Function remoteGetZip