kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #22204
Re: Libcurl patch.
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.charras@xxxxxxxxxx>
>>>> - * Copyright (C) 2013 SoftPLC Corporation, Dick Hollenbeck <dick@xxxxxxxxxxx>
>>>> + * Copyright (C) 2013-2016 SoftPLC Corporation, Dick Hollenbeck <dick@xxxxxxxxxxx>
>>>> * 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@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>> More help : https://help.launchpad.net/ListHelp
>>>
Follow ups
References
