On 12/26/2015 11:20 AM, Chris Pavlina wrote: > Fair enough, I just don't see how it makes sense. Even if > bitmap2component doesn't need libcurl, loading it anyway uses rather few > resources, and simplifies the code, reducing the number of places we can > expect to find bugs. Dynamically loading libraries like that is > generally something people avoid unless they really need to - even if > bitmap2component doesn't require libcurl, I promise you it doesn't mind > having it ;) and nobody's really hurting for the 100ms reduction in load > time either ;) > > Just my two cents, anyway. Not something I feel particularly strongly > about, but it seems a little bit silly to me.
I take full responsibility for this. I was the one who asked Dick to code it this way. After I saw his patch, I realized that I should have just told him to keep the runtime loading. For now I will leave it as is. I can revisit it should it prove to be problematic. > > Agreed about the shared object, but that's a separate topic. > > > On Sat, Dec 26, 2015 at 11:14:26AM -0500, Wayne Stambaugh wrote: >> On 12/26/2015 11:00 AM, Chris Pavlina wrote: >>> On Sat, Dec 26, 2015 at 10:50:08AM -0500, Wayne Stambaugh wrote: >>>> [snip] One thing he did that I >>>> asked him to do was make libcurl dynamically loadable since it isn't >>>> always necessary to load it at run time. [snip] >>> >>> Is there a real noticeable advantage to this, though? It seems to me >>> that it's just adding complexity - and possible bug space - for a >>> theoretical improvement in load time. I certainly didn't notice my load >>> time increase with the original libcurl patch. >> >> The problem is that libcurl gets loaded due to the kiface design even in >> the apps that don't use the github plugin (according to Dick, I didn't >> have time to confirm this but I'm confident this is the case) so loading >> libcurl at runtime for the bitmap2component app doesn't make a lot of >> sense. We could change it back to load at runtime if there are issues. >> We probably should push this into a kicad shared object instead of >> statically linking which would solve some of the kiface/app >> initialization issues that Dick discovered. There is some stub code in >> common/CMakeList.txt already in place it's just an issue of finding the >> time to implement it. >> >>> >>> >>>> windows and linux but I would like one of our osx devs to please test it >>>> to make sure it works on osx when you get a chance. >>>> >>>> Thanks, >>>> >>>> Wayne >>> >>>> === modified file 'common/CMakeLists.txt' >>>> --- common/CMakeLists.txt 2015-12-21 20:30:33 +0000 >>>> +++ common/CMakeLists.txt 2015-12-22 00:00:19 +0000 >>>> @@ -283,7 +283,10 @@ >>>> 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 >>>> >>>> === modified file 'common/footprint_info.cpp' >>>> --- common/footprint_info.cpp 2015-11-11 18:35:26 +0000 >>>> +++ common/footprint_info.cpp 2015-12-22 00:00:19 +0000 >>>> @@ -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.char...@wanadoo.fr> >>>> - * Copyright (C) 2013 SoftPLC Corporation, Dick Hollenbeck >>>> <d...@softplc.com> >>>> + * Copyright (C) 2013-2016 SoftPLC Corporation, Dick Hollenbeck >>>> <d...@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 @@ >>>> } >>>> >>>> >>>> -#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 @@ >>>> // 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 @@ >>>> >>>> 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 @@ >>>> break; >>>> } >>>> >>>> - int jobz = JOBZ; >>>> - >>>> - if( i + jobz >= nicknames.size() ) >>>> + if( i + jobz >= nicknames.size() ) // on the last iteration >>>> of this for(;;) >>>> { >>>> 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 @@ >>>> { >>>> threads[i].join(); >>>> } >>>> -#else >>>> - loader_job( &nicknames[0], nicknames.size() ); >>>> -#endif >>>> >>>> m_list.sort(); >>>> } >>>> >>>> === modified file 'common/kicad_curl/kicad_curl.cpp' >>>> --- common/kicad_curl/kicad_curl.cpp 2015-12-22 14:19:00 +0000 >>>> +++ common/kicad_curl/kicad_curl.cpp 2015-12-24 15:04:12 +0000 >>>> @@ -22,51 +22,190 @@ >>>> * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA >>>> */ >>>> >>>> +#include <wx/log.h> >>>> +#include <wx/dynlib.h> >>>> + >>>> +#include <macros.h> >>>> #include <kicad_curl/kicad_curl.h> >>>> - >>>> -bool KICAD_CURL::Init() >>>> -{ >>>> - if ( curl_global_init( CURL_GLOBAL_ALL ) != CURLE_OK ) >>>> - { >>>> - return false; >>>> - } >>>> - else >>>> - { >>>> - m_initialized = true; >>>> - return true; >>>> +#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 >>>> +{ >>>> + 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() >>>> +{ >>>> + // 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", >>>> + static_cast<const char*>( >>>> canonicalName ) ); >>>> + THROW_IO_ERROR( msg ); >>>> + } >>>> + >>>> + // get addresses. >>>> + >>>> + for( unsigned i=0; i < DIM(dyn_funcs); ++i ) >>>> + { >>>> + *dyn_funcs[i].address = dso.GetSymbol( dyn_funcs[i].name >>>> ); >>>> + >>>> + 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(); >>>> -} >>>> - >>>> - >>>> -std::string KICAD_CURL::GetVersion() >>>> -{ >>>> - return std::string( curl_version() ); >>>> + /* >>>> + >>>> + 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::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; >>>> >>>> if( info->version ) >>>> { >>>> - res += "libcurl version: " + std::string(info->version); >>>> + res += "libcurl version: " + std::string( info->version ); >>>> } >>>> >>>> res += " ("; >>>> + >>>> if( info->features & CURL_VERSION_SSL ) >>>> { >>>> res += "with SSL - "; >>>> - res += std::string(info->ssl_version); >>>> + res += std::string( info->ssl_version ); >>>> } >>>> else >>>> { >>>> @@ -76,5 +215,3 @@ >>>> >>>> return res; >>>> } >>>> - >>>> -bool KICAD_CURL::m_initialized = false; >>>> \ No newline at end of file >>>> >>>> === modified file 'common/kicad_curl/kicad_curl_easy.cpp' >>>> --- common/kicad_curl/kicad_curl_easy.cpp 2015-12-22 14:19:00 +0000 >>>> +++ common/kicad_curl/kicad_curl_easy.cpp 2015-12-23 20:21:01 +0000 >>>> @@ -30,134 +30,70 @@ >>>> #include <sstream> >>>> #include <richio.h> >>>> >>>> -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(); >>>> - >>>> - if( m_CURL == NULL ) >>>> + >>>> +static size_t write_callback( void* contents, size_t size, size_t nmemb, >>>> void* userp ) >>>> +{ >>>> + size_t realsize = size * nmemb; >>>> + >>>> + 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 ); >>>> - m_Buffer.Size = 0; >>>> - >>>> - curl_easy_setopt( m_CURL, CURLOPT_WRITEFUNCTION, write_callback ); >>>> - curl_easy_setopt( m_CURL, CURLOPT_WRITEDATA, (void *)&m_Buffer ); >>>> + KICAD_CURL::easy_setopt( m_CURL, CURLOPT_WRITEFUNCTION, >>>> write_callback ); >>>> + KICAD_CURL::easy_setopt( m_CURL, CURLOPT_WRITEDATA, (void*) &m_buffer >>>> ); >>>> } >>>> >>>> >>>> KICAD_CURL_EASY::~KICAD_CURL_EASY() >>>> { >>>> - free(m_Buffer.Payload); >>>> - 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; >>>> + if( m_headers ) >>>> + KICAD_CURL::slist_free_all( m_headers ); >>>> + >>>> + KICAD_CURL::easy_cleanup( m_CURL ); >>>> } >>>> >>>> >>>> void KICAD_CURL_EASY::Perform() >>>> { >>>> - if( m_headers != NULL ) >>>> - { >>>> - curl_easy_setopt( m_CURL, CURLOPT_HTTPHEADER, m_headers ); >>>> - } >>>> - >>>> - if( m_Buffer.Size > 0 ) >>>> - { >>>> - free( m_Buffer.Payload ); >>>> - m_Buffer.Payload = (char*)malloc( 1 ); >>>> - m_Buffer.Size = 0; >>>> - } >>>> - >>>> - CURLcode res = curl_easy_perform( m_CURL ); >>>> + if( m_headers ) >>>> + { >>>> + KICAD_CURL::easy_setopt( m_CURL, CURLOPT_HTTPHEADER, m_headers ); >>>> + } >>>> + >>>> + // bonus: retain worst case memory allocation, should re-use occur >>>> + m_buffer.clear(); >>>> + >>>> + CURLcode res = KICAD_CURL::easy_perform( m_CURL ); >>>> + >>>> if( res != CURLE_OK ) >>>> { >>>> - wxString msg = wxString::Format( >>>> - _( "CURL Request Failed: %s" ), >>>> - GetErrorText( res ) ); >>>> - >>>> + std::string msg = StrPrintf( "curl_easy_perform()=%d: %s", >>>> + res, GetErrorText( res ).c_str() ); >>>> THROW_IO_ERROR( msg ); >>>> } >>>> -} >>>> \ No newline at end of file >>>> +} >>>> >>>> === modified file 'common/pgm_base.cpp' >>>> --- common/pgm_base.cpp 2015-12-21 20:30:33 +0000 >>>> +++ common/pgm_base.cpp 2015-12-22 00:00:19 +0000 >>>> @@ -283,7 +283,6 @@ >>>> PGM_BASE::~PGM_BASE() >>>> { >>>> destroy(); >>>> - KICAD_CURL::Cleanup(); >>>> } >>>> >>>> >>>> @@ -291,6 +290,8 @@ >>>> { >>>> // 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 @@ >>>> 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; >>>> } >>>> >>>> >>>> === modified file 'include/kicad_curl/kicad_curl.h' >>>> --- include/kicad_curl/kicad_curl.h 2015-12-22 14:19:00 +0000 >>>> +++ include/kicad_curl/kicad_curl.h 2015-12-24 14:28:51 +0000 >>>> @@ -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 @@ >>>> * 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 @@ >>>> * 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 @@ >>>> */ >>>> 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_ >>>> \ No newline at end of file >>>> +#endif // KICAD_CURL_H_ >>>> >>>> === modified file 'include/kicad_curl/kicad_curl_easy.h' >>>> --- include/kicad_curl/kicad_curl_easy.h 2015-12-22 14:19:00 +0000 >>>> +++ include/kicad_curl/kicad_curl_easy.h 2015-12-23 20:25:18 +0000 >>>> @@ -27,7 +27,7 @@ >>>> /* >>>> * KICAD_CURL_EASY.h must included before wxWidgets because on Windows, >>>> * wxWidgets ends up including windows.h before winsocks2.h inside curl >>>> - * this causes build warnings >>>> + * this causes build warnings >>>> * Because we are before wx, we must explicitly define we are building >>>> with unicode >>>> * wxWidgets defaults to supporting unicode now, so this should be safe. >>>> */ >>>> @@ -42,19 +42,9 @@ >>>> #endif >>>> >>>> >>>> +#include <string> >>>> #include <curl/curl.h> >>>> -#include <string> >>>> - >>>> -/** >>>> - * 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; >>>> -}; >>>> +#include <kicad_curl/kicad_curl.h> >>>> >>>> >>>> /** >>>> @@ -67,9 +57,10 @@ >>>> * Here is a small example usage: >>>> * @code >>>> * KICAD_CURL_EASY curl; >>>> - * curl.SetURL("http://github.com"); >>>> - * curl.SetUserAgent("KiCad-EDA"); >>>> - * curl.SetHeader("Accept", "application/json"); >>>> + * >>>> + * curl.SetURL( "http://github.com" ); >>>> + * curl.SetUserAgent( <http-client-indentifier> ); >>>> + * curl.SetHeader( "Accept", "application/json" ); >>>> * curl.Perform(); >>>> * @endcode >>>> */ >>>> @@ -95,7 +86,11 @@ >>>> * @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 @@ >>>> * @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 @@ >>>> * @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 @@ >>>> * @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 >>>> @@ -143,24 +162,23 @@ >>>> * @return CURLcode - CURL error code, will return CURLE_OK unless a >>>> problem was encountered >>>> */ >>>> 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 >>>> - * >>>> - * @return KICAD_EASY_CURL_BUFFER* - pointer to buffer >>>> + * returns a const reference to the recevied data 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; >>>> - struct KICAD_EASY_CURL_BUFFER m_Buffer; >>>> + CURL* m_CURL; >>>> + curl_slist* m_headers; >>>> + std::string m_buffer; >>>> }; >>>> >>>> -#endif // KICAD_CURL_EASY_H_ >>>> \ No newline at end of file >>>> +#endif // KICAD_CURL_EASY_H_ >>>> >>>> === modified file 'pcbnew/CMakeLists.txt' >>>> --- pcbnew/CMakeLists.txt 2015-12-21 14:55:31 +0000 >>>> +++ pcbnew/CMakeLists.txt 2015-12-23 20:33:58 +0000 >>>> @@ -426,11 +426,11 @@ >>>> 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 @@ >>>> gal >>>> lib_dxf >>>> idf3 >>>> + ${wxWidgets_LIBRARIES} >>>> ${GITHUB_PLUGIN_LIBRARIES} >>>> - ${wxWidgets_LIBRARIES} >>>> ${GDI_PLUS_LIBRARIES} >>>> ${PYTHON_LIBRARIES} >>>> ${Boost_LIBRARIES} # must follow GITHUB >>>> >>>> === modified file 'pcbnew/github/github_getliblist.cpp' >>>> --- pcbnew/github/github_getliblist.cpp 2015-12-22 14:19:00 +0000 >>>> +++ pcbnew/github/github_getliblist.cpp 2015-12-23 20:41:03 +0000 >>>> @@ -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 (*aFilter)( const wxString& aData ) ) >>>> { >>>> std::string fullURLCommand; >>>> + >>>> strcpy( m_option_string, "text/html" ); >>>> >>>> wxString repoURL = m_repoURL; >>>> @@ -95,6 +96,7 @@ >>>> 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 >>>> @@ -212,16 +214,15 @@ >>>> >>>> wxLogDebug( wxT( "Attempting to download: " ) + aFullURLCommand ); >>>> >>>> - kcurl.SetURL(aFullURLCommand); >>>> - kcurl.SetUserAgent("KiCad-EDA"); >>>> - kcurl.SetHeader("Accept", m_option_string); >>>> - kcurl.SetFollowRedirects(true); >>>> + kcurl.SetURL( aFullURLCommand ); >>>> + 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.assign( kcurl.GetBuffer()->Payload, >>>> kcurl.GetBuffer()->Size ); >>>> + m_image = kcurl.GetBuffer(); >>>> return true; >>>> } >>>> catch( const IO_ERROR& ioe ) >>>> @@ -229,8 +230,8 @@ >>>> if( aMsgError ) >>>> { >>>> UTF8 fmt( _( "Error fetching JSON data from URL >>>> '%s'.\nReason: '%s'" ) ); >>>> - >>>> - std::string msg = StrPrintf( fmt.c_str(), >>>> + >>>> + std::string msg = StrPrintf( fmt.c_str(), >>>> aFullURLCommand.c_str(), >>>> TO_UTF8( ioe.errorText ) ); >>>> >>>> @@ -238,4 +239,4 @@ >>>> } >>>> return false; >>>> } >>>> -} >>>> \ No newline at end of file >>>> +} >>>> >>>> === modified file 'pcbnew/github/github_plugin.cpp' >>>> --- pcbnew/github/github_plugin.cpp 2015-12-22 14:19:00 +0000 >>>> +++ pcbnew/github/github_plugin.cpp 2015-12-23 20:40:56 +0000 >>>> @@ -38,7 +38,7 @@ >>>> mechanism can be found, or github gets more servers. But note that the >>>> occasionally >>>> slow response is the exception rather than the norm. Normally the >>>> response is >>>> down around a 1/3 of a second. The information we would use is in the >>>> header >>>> -named "Last-Modified" as seen below. >>>> +named "Last-Modified" as seen below. >>>> >>>> >>>> HTTP/1.1 200 OK >>>> @@ -61,10 +61,9 @@ >>>> 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 @@ >>>> >>>> const wxString GITHUB_PLUGIN::PluginName() const >>>> { >>>> - return wxT( "Github" ); >>>> + return "Github"; >>>> } >>>> >>>> >>>> @@ -391,7 +390,7 @@ >>>> >>>> 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 @@ >>>> } >>>> >>>> // 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 @@ >>>> } >>>> >>>> >>>> -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 @@ >>>> // 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 @@ >>>> { >>>> 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,31 @@ >>>> >>>> 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.SetHeader("Accept", "application/zip"); >>>> - kcurl.SetFollowRedirects(true); >>>> + kcurl.SetURL( zip_url.c_str() ); >>>> + 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.assign( kcurl.GetBuffer()->Payload, >>>> kcurl.GetBuffer()->Size ); >>>> + m_zip_image = kcurl.GetBuffer(); >>>> } >>>> 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(), >>>> + std::string msg = StrPrintf( fmt.c_str(), >>>> + errorcmd, >>>> + zip_url.c_str(), >>>> TO_UTF8( aRepoURL ), >>>> - TO_UTF8( ioe.errorText ) ); >>>> + TO_UTF8( ioe.errorText ) >>>> + ); >>>> >>>> THROW_IO_ERROR( msg ); >>>> } >>>> @@ -565,7 +568,7 @@ >>>> try >>>> { >>>> wxArrayString fps = gh.FootprintEnumerate( >>>> - wxT( "https://github.com/liftoff-sr/pretty_footprints" ), >>>> + "https://github.com/liftoff-sr/pretty_footprints", >>>> NULL >>>> ); >>>> >>>> >>>> === modified file 'pcbnew/github/github_plugin.h' >>>> --- pcbnew/github/github_plugin.h 2015-12-21 20:30:33 +0000 >>>> +++ pcbnew/github/github_plugin.h 2015-12-22 00:00:19 +0000 >>>> @@ -210,7 +210,7 @@ >>>> * @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 >>>> >>> >>>> _______________________________________________ >>>> Mailing list: https://launchpad.net/~kicad-developers >>>> Post to : kicad-developers@lists.launchpad.net >>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>> More help : https://help.launchpad.net/ListHelp >>> _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp