kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #22203
Re: Libcurl patch.
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.
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