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