From e842d711c620997c2a639d6001a7ccd7230e783b Mon Sep 17 00:00:00 2001 From: Dick Hollenbeck Date: Tue, 10 Dec 2013 17:41:34 -0600 Subject: [PATCH] FIX: make LEGACY_PLUGIN re-entrant. extern "C" strtok_r() put conditionally into libcommon. --- CMakeModules/PerformFeatureChecks.cmake | 1 + CMakeModules/config.h.cmake | 2 + common/CMakeLists.txt | 4 + common/footprint_info.cpp | 163 +++++++++--------------- include/kicad_string.h | 5 + pcbnew/legacy_plugin.cpp | 96 ++++++++------ 6 files changed, 127 insertions(+), 144 deletions(-) diff --git a/CMakeModules/PerformFeatureChecks.cmake b/CMakeModules/PerformFeatureChecks.cmake index 86ad7c5a84..330cc2cb8b 100644 --- a/CMakeModules/PerformFeatureChecks.cmake +++ b/CMakeModules/PerformFeatureChecks.cmake @@ -77,6 +77,7 @@ macro(perform_feature_checks) check_symbol_exists(strcasecmp "strings.h" HAVE_STRCASECMP) check_symbol_exists(strncasecmp "string.h" HAVE_STRNCASECMP) check_symbol_exists(strncasecmp "strings.h" HAVE_STRNCASECMP) + check_symbol_exists( strtok_r "string.h" HAVE_STRTOKR ) # Some platforms define malloc and free in malloc.h instead of stdlib.h. check_symbol_exists(malloc "stdlib.h" MALLOC_IN_STDLIB_H) diff --git a/CMakeModules/config.h.cmake b/CMakeModules/config.h.cmake index da209b0b39..16f40c83be 100644 --- a/CMakeModules/config.h.cmake +++ b/CMakeModules/config.h.cmake @@ -7,6 +7,8 @@ #cmakedefine HAVE_STRNCASECMP +#cmakedefine HAVE_STRTOKR // spelled odly to differ from wx's similar test + // Handle platform differences in math.h #cmakedefine HAVE_MATH_H diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index ba0e2b0029..b70b8b4660 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -149,6 +149,10 @@ set(COMMON_SRCS zoom.cpp ) +if( NOT HAVE_STRTOKR ) + set( COMMON_SRCS ${COMMON_SRCS} strtok_r.c ) +endif() + enable_language(C CXX ASM) set_source_files_properties(system/fcontext.s PROPERTIES COMPILE_FLAGS "-x assembler-with-cpp") diff --git a/common/footprint_info.cpp b/common/footprint_info.cpp index 105bd19727..7611968ba5 100644 --- a/common/footprint_info.cpp +++ b/common/footprint_info.cpp @@ -53,9 +53,20 @@ /* -wxString ToHTML( const IO_ERROR** aList, int aCount ) +static wxString ToHTMLFragment( const IO_ERROR* aDerivative ) { - wxString msg = wxT( "" ); + @todo + + 1) change up IO_ERROR so it keeps linenumbers, source file name and + error message in separate strings. + + 2) Add a summarizing virtual member like + virtual wxString What() + to combine all portions of an IO_ERROR's text into a single wxString. + + 3) Do same for PARSE_ERROR. + + 4) Add a "reason or error category" to IO_ERROR and thereby also PARSE_ERROR? msg += " @@ -175,11 +186,6 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( wxArrayString& aFootprintLibNames ) #else // yes USE_FP_LIB_TABLE, by all means: - -#if USE_WORKER_THREADS //--------------------------------------------------------------------- - -#define USE_WORKER_THREADS 1 // 1:yes, 0:no. use worker threads to load libraries - #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 @@ -191,7 +197,6 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( wxArrayString& aFootprintLibNames ) // 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 );) @@ -225,7 +230,7 @@ void FOOTPRINT_LIST::loader_job( const wxString* aNicknameList, int aJobZ ) } catch( const PARSE_ERROR& pe ) { - // push_back is not thread safe, use the lock the MUTEX. + // m_errors.push_back is not thread safe, lock its MUTEX. MUTLOCK lock( m_errors_lock ); ++m_error_count; // modify only under lock @@ -244,7 +249,7 @@ void FOOTPRINT_LIST::loader_job( const wxString* aNicknameList, int aJobZ ) // worker threads. catch( const std::exception& se ) { - // this is a round about way to do this, but who knows what THROW_IO_ERROR() + // This is a round about way to do this, but who knows what THROW_IO_ERROR() // may be tricked out to do someday, keep it in the game. try { @@ -261,8 +266,6 @@ void FOOTPRINT_LIST::loader_job( const wxString* aNicknameList, int aJobZ ) } } -#endif // USE_WORKER_THREADS --------------------------------------------------- - bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* aNickname ) { @@ -275,120 +278,74 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a m_errors.clear(); m_list.clear(); - std::vector< wxString > nicknames; + if( aNickname ) + // single footprint + loader_job( aNickname, 1 ); + else + { + std::vector< wxString > nicknames; - if( !aNickname ) // do all of them nicknames = aTable->GetLogicalLibs(); - else - // single footprint - nicknames.push_back( *aNickname ); #if USE_WORKER_THREADS - // Something which will not invoke a thread copy constructor, one of many ways obviously: - typedef boost::ptr_vector< boost::thread > MYTHREADS; + // Something which will not invoke a thread copy constructor, one of many ways obviously: + typedef boost::ptr_vector< boost::thread > MYTHREADS; - MYTHREADS threads; + MYTHREADS 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= NTOLERABLE_ERRORS ) + // 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= NTOLERABLE_ERRORS ) + { + // abort the remaining nicknames. + retv = false; + break; + } + + int jobz = JOBZ; + + if( i + jobz >= nicknames.size() ) + { + jobz = nicknames.size() - i; + + // Only a little bit to do, I'll do it myself, on current thread. + loader_job( &nicknames[i], jobz ); + } + else + { + // Delegate the job to a worker thread created here. + threads.push_back( new boost::thread( &FOOTPRINT_LIST::loader_job, + this, &nicknames[i], jobz ) ); + } + + i += jobz; } - int jobz = JOBZ; - - if( i + jobz >= nicknames.size() ) + // Wait for all the worker threads to complete, it does not matter in what order + // we wait for them as long as a full sweep is made. Think of the great race, + // everyone must finish. + for( unsigned i=0; iFootprintEnumerate( nickname ); - - for( unsigned i=0; i m( aTable->FootprintLoad( nickname, fpnames[i] ) ); - - // we're loading what we enumerated, all must be there. - wxASSERT( m.get() ); - - FOOTPRINT_INFO* fpinfo = new FOOTPRINT_INFO(); - - fpinfo->SetNickname( nickname ); - - fpinfo->m_Module = fpnames[i]; - fpinfo->m_padCount = m->GetPadCount( MODULE::DO_NOT_INCLUDE_NPTH ); - fpinfo->m_KeyWord = m->GetKeywords(); - fpinfo->m_Doc = m->GetDescription(); - - AddItem( fpinfo ); - } - } - catch( const PARSE_ERROR& pe ) - { - m_errors.push_back( new IO_ERROR( pe ) ); - retv = false; - } - catch( const IO_ERROR& ioe ) - { - m_errors.push_back( new IO_ERROR( ioe ) ); - retv = false; - } - } - - m_list.sort(); - - return retv; - -#endif } - #endif // USE_FP_LIB_TABLE diff --git a/include/kicad_string.h b/include/kicad_string.h index 49d23742c1..314e803a8f 100644 --- a/include/kicad_string.h +++ b/include/kicad_string.h @@ -164,4 +164,9 @@ wxString GetIllegalFileNameWxChars(); */ bool ReplaceIllegalFileNameChars( std::string* aName ); +#ifndef HAVE_STRTOKR +// common/strtok_r.c optionally: +extern "C" char* strtok_r( char* str, const char* delim, char** nextp ); +#endif + #endif // KICAD_STRING_H_ diff --git a/pcbnew/legacy_plugin.cpp b/pcbnew/legacy_plugin.cpp index c941c00210..11afe8aa0f 100644 --- a/pcbnew/legacy_plugin.cpp +++ b/pcbnew/legacy_plugin.cpp @@ -409,6 +409,7 @@ void LEGACY_PLUGIN::checkVersion() void LEGACY_PLUGIN::loadGENERAL() { char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -417,7 +418,7 @@ void LEGACY_PLUGIN::loadGENERAL() if( TESTLINE( "Units" ) ) { // what are the engineering units of the lengths in the BOARD? - data = strtok( line + SZ("Units"), delims ); + data = strtok_r( line + SZ("Units"), delims, &saveptr ); if( !strcmp( data, "mm" ) ) { @@ -520,6 +521,7 @@ void LEGACY_PLUGIN::loadSHEET() char buf[260]; TITLE_BLOCK tb; char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -529,7 +531,7 @@ void LEGACY_PLUGIN::loadSHEET() // width and height are in 1/1000th of an inch, always PAGE_INFO page; - char* sname = strtok( line + SZ( "Sheet" ), delims ); + char* sname = strtok_r( line + SZ( "Sheet" ), delims, &saveptr ); if( sname ) { @@ -541,9 +543,9 @@ void LEGACY_PLUGIN::loadSHEET() THROW_IO_ERROR( m_error ); } - char* width = strtok( NULL, delims ); - char* height = strtok( NULL, delims ); - char* orient = strtok( NULL, delims ); + char* width = strtok_r( NULL, delims, &saveptr ); + char* height = strtok_r( NULL, delims, &saveptr ); + char* orient = strtok_r( NULL, delims, &saveptr ); // only parse the width and height if page size is custom ("User") if( wname == PAGE_INFO::Custom ) @@ -634,6 +636,7 @@ void LEGACY_PLUGIN::loadSETUP() BOARD_DESIGN_SETTINGS bds = m_board->GetDesignSettings(); ZONE_SETTINGS zs = m_board->GetZoneSettings(); char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -671,13 +674,13 @@ void LEGACY_PLUGIN::loadSETUP() LAYER_NUM layer = layerParse( line + SZ( "Layer[" ), &data ); - data = strtok( (char*) data+1, delims ); // +1 for ']' + data = strtok_r( (char*) data+1, delims, &saveptr ); // +1 for ']' if( data ) { wxString layerName = FROM_UTF8( data ); m_board->SetLayerName( layer, layerName ); - data = strtok( NULL, delims ); + data = strtok_r( NULL, delims, &saveptr ); if( data ) // optional in old board files { LAYER_T type = LAYER::ParseType( data ); @@ -747,7 +750,7 @@ void LEGACY_PLUGIN::loadSETUP() BIU drill = 0; BIU diameter = biuParse( line + SZ( "ViaSizeList" ), &data ); - data = strtok( (char*) data, delims ); + data = strtok_r( (char*) data, delims, &saveptr ); if( data ) // DRILL may not be present ? drill = biuParse( data ); @@ -929,7 +932,8 @@ void LEGACY_PLUGIN::loadSETUP() void LEGACY_PLUGIN::LoadMODULE( MODULE* aModule ) { - char* line; + char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -989,7 +993,7 @@ void LEGACY_PLUGIN::LoadMODULE( MODULE* aModule ) long edittime = hexParse( data, &data ); time_t timestamp = hexParse( data, &data ); - data = strtok( (char*) data+1, delims ); + data = strtok_r( (char*) data+1, delims, &saveptr ); // data is now a two character long string // Note: some old files do not have this field @@ -1061,7 +1065,7 @@ void LEGACY_PLUGIN::LoadMODULE( MODULE* aModule ) else if( TESTLINE( "AR" ) ) // Alternate Reference { // e.g. "AR /47BA2624/45525076" - data = strtok( line + SZ( "AR" ), delims ); + data = strtok_r( line + SZ( "AR" ), delims, &saveptr ); if( data ) aModule->SetPath( FROM_UTF8( data ) ); } @@ -1151,6 +1155,7 @@ void LEGACY_PLUGIN::loadPAD( MODULE* aModule ) { auto_ptr pad( new D_PAD( aModule ) ); char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -1239,17 +1244,17 @@ void LEGACY_PLUGIN::loadPAD( MODULE* aModule ) PAD_SHAPE_T drShape = PAD_CIRCLE; - data = strtok( (char*) data, delims ); + data = strtok_r( (char*) data, delims, &saveptr ); if( data ) // optional shape { if( data[0] == 'O' ) { drShape = PAD_OVAL; - data = strtok( NULL, delims ); + data = strtok_r( NULL, delims, &saveptr ); drill_x = biuParse( data ); - data = strtok( NULL, delims ); + data = strtok_r( NULL, delims, &saveptr ); drill_y = biuParse( data ); } } @@ -1266,7 +1271,7 @@ void LEGACY_PLUGIN::loadPAD( MODULE* aModule ) PAD_ATTR_T attribute; - data = strtok( line + SZ( "At" ), delims ); + data = strtok_r( line + SZ( "At" ), delims, &saveptr ); if( !strcmp( data, "SMD" ) ) attribute = PAD_SMD; @@ -1277,8 +1282,8 @@ void LEGACY_PLUGIN::loadPAD( MODULE* aModule ) else attribute = PAD_STANDARD; - data = strtok( NULL, delims ); // skip BufCar - data = strtok( NULL, delims ); + data = strtok_r( NULL, delims, &saveptr ); // skip BufCar + data = strtok_r( NULL, delims, &saveptr ); LAYER_MSK layer_mask = hexParse( data ); @@ -1532,6 +1537,7 @@ void LEGACY_PLUGIN::loadMODULE_TEXT( TEXTE_MODULE* aText ) const char* data; const char* txt_end; const char* line = m_reader->Line(); // current (old) line + char* saveptr; // sscanf( line + 1, "%d %d %d %d %d %d %d %s %s %d %s", // &type, &m_Pos0.x, &m_Pos0.y, &m_Size.y, &m_Size.x, @@ -1571,14 +1577,14 @@ void LEGACY_PLUGIN::loadMODULE_TEXT( TEXTE_MODULE* aText ) // after switching to strtok, there's no easy coming back because of the // embedded nul(s?) placed to the right of the current field. // (that's the reason why strtok was deprecated...) - char* mirror = strtok( (char*) data, delims ); - char* hide = strtok( NULL, delims ); - char* tmp = strtok( NULL, delims ); + char* mirror = strtok_r( (char*) data, delims, &saveptr ); + char* hide = strtok_r( NULL, delims, &saveptr ); + char* tmp = strtok_r( NULL, delims, &saveptr ); LAYER_NUM layer = tmp ? layerParse( tmp ) : SILKSCREEN_N_FRONT; - char* italic = strtok( NULL, delims ); + char* italic = strtok_r( NULL, delims, &saveptr ); - char* hjust = strtok( (char*) txt_end, delims ); - char* vjust = strtok( NULL, delims ); + char* hjust = strtok_r( (char*) txt_end, delims, &saveptr ); + char* vjust = strtok_r( NULL, delims, &saveptr ); if( type != TEXTE_MODULE::TEXT_is_REFERENCE && type != TEXTE_MODULE::TEXT_is_VALUE ) @@ -1697,7 +1703,9 @@ void LEGACY_PLUGIN::loadPCB_LINE() */ auto_ptr dseg( new DRAWSEGMENT( m_board ) ); - char* line; + + char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -1727,8 +1735,8 @@ void LEGACY_PLUGIN::loadPCB_LINE() BIU x = 0; BIU y; - data = strtok( line + SZ( "De" ), delims ); - for( int i = 0; data; ++i, data = strtok( NULL, delims ) ) + data = strtok_r( line + SZ( "De" ), delims, &saveptr ); + for( int i = 0; data; ++i, data = strtok_r( NULL, delims, &saveptr ) ) { switch( i ) { @@ -1864,7 +1872,8 @@ void LEGACY_PLUGIN::loadPCB_TEXT() TEXTE_PCB* pcbtxt = new TEXTE_PCB( m_board ); m_board->Add( pcbtxt, ADD_APPEND ); - char* line; + char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -1928,9 +1937,9 @@ void LEGACY_PLUGIN::loadPCB_TEXT() LAYER_NUM layer = layerParse( line + SZ( "De" ), &data ); int notMirrored = intParse( data, &data ); time_t timestamp = hexParse( data, &data ); - char* style = strtok( (char*) data, delims ); - char* hJustify = strtok( NULL, delims ); - char* vJustify = strtok( NULL, delims ); + char* style = strtok_r( (char*) data, delims, &saveptr ); + char* hJustify = strtok_r( NULL, delims, &saveptr ); + char* vJustify = strtok_r( NULL, delims, &saveptr ); pcbtxt->SetMirrored( !notMirrored ); pcbtxt->SetTimeStamp( timestamp ); @@ -1968,6 +1977,7 @@ void LEGACY_PLUGIN::loadPCB_TEXT() void LEGACY_PLUGIN::loadTrackList( int aStructType ) { char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -1993,7 +2003,7 @@ void LEGACY_PLUGIN::loadTrackList( int aStructType ) BIU width = biuParse( data, &data ); // optional 7th drill parameter (must be optional in an old format?) - data = strtok( (char*) data, delims ); + data = strtok_r( (char*) data, delims, &saveptr ); BIU drill = data ? biuParse( data ) : -1; // SetDefault() if < 0 @@ -2188,6 +2198,7 @@ void LEGACY_PLUGIN::loadZONE_CONTAINER() bool sawCorner = false; char buf[1024]; char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -2240,7 +2251,7 @@ void LEGACY_PLUGIN::loadZONE_CONTAINER() { // e.g. "ZAux 7 E" int ignore = intParse( line + SZ( "ZAux" ), &data ); - char* hopt = strtok( (char*) data, delims ); + char* hopt = strtok_r( (char*) data, delims, &saveptr ); if( !hopt ) { @@ -2284,27 +2295,27 @@ void LEGACY_PLUGIN::loadZONE_CONTAINER() { zc->SetIsKeepout( true ); // e.g. "ZKeepout tracks N vias N pads Y" - data = strtok( line + SZ( "ZKeepout" ), delims ); + data = strtok_r( line + SZ( "ZKeepout" ), delims, &saveptr ); while( data ) { if( !strcmp( data, "tracks" ) ) { - data = strtok( NULL, delims ); + data = strtok_r( NULL, delims, &saveptr ); zc->SetDoNotAllowTracks( data && *data == 'N' ); } else if( !strcmp( data, "vias" ) ) { - data = strtok( NULL, delims ); + data = strtok_r( NULL, delims, &saveptr ); zc->SetDoNotAllowVias( data && *data == 'N' ); } else if( !strcmp( data, "copperpour" ) ) { - data = strtok( NULL, delims ); + data = strtok_r( NULL, delims, &saveptr ); zc->SetDoNotAllowCopperPour( data && *data == 'N' ); } - data = strtok( NULL, delims ); + data = strtok_r( NULL, delims, &saveptr ); } } @@ -2335,7 +2346,7 @@ void LEGACY_PLUGIN::loadZONE_CONTAINER() { // e.g. "ZClearance 40 I" BIU clearance = biuParse( line + SZ( "ZClearance" ), &data ); - char* padoption = strtok( (char*) data, delims ); // data: " I" + char* padoption = strtok_r( (char*) data, delims, &saveptr ); // data: " I" ZoneConnection popt; switch( *padoption ) @@ -2447,7 +2458,9 @@ void LEGACY_PLUGIN::loadZONE_CONTAINER() void LEGACY_PLUGIN::loadDIMENSION() { auto_ptr dim( new DIMENSION( m_board ) ); - char* line; + + char* line; + char* saveptr; while( ( line = READLINE( m_reader ) ) != NULL ) { @@ -2504,7 +2517,7 @@ void LEGACY_PLUGIN::loadDIMENSION() BIU height = biuParse( data, &data ); BIU thickn = biuParse( data, &data ); double orient = degParse( data, &data ); - char* mirror = strtok( (char*) data, delims ); + char* mirror = strtok_r( (char*) data, delims, &saveptr ); // This sets both DIMENSION's position and internal m_Text's. // @todo: But why do we even know about internal m_Text? @@ -3962,6 +3975,7 @@ void LP_CACHE::Load() void LP_CACHE::ReadAndVerifyHeader( LINE_READER* aReader ) { char* line = aReader->ReadLine(); + char* saveptr; if( !line ) goto L_bad_library; @@ -3973,7 +3987,7 @@ void LP_CACHE::ReadAndVerifyHeader( LINE_READER* aReader ) { if( TESTLINE( "Units" ) ) { - const char* units = strtok( line + SZ( "Units" ), delims ); + const char* units = strtok_r( line + SZ( "Units" ), delims, &saveptr ); if( !strcmp( units, "mm" ) ) {