FIX: make LEGACY_PLUGIN re-entrant. extern "C" strtok_r() put conditionally into libcommon.

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)

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
 

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")
 

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( "<table>" );
+    @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,14 +278,15 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a
     m_errors.clear();
     m_list.clear();
 
+    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
 
@@ -309,7 +313,6 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a
                 jobz = nicknames.size() - i;
 
                 // Only a little bit to do, I'll do it myself, on current thread.
-            // This is the path for a single footprint also.
                 loader_job( &nicknames[i], jobz );
             }
             else
@@ -329,8 +332,12 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a
         {
             threads[i].join();
         }
+#else
+        loader_job( &nicknames[0], nicknames.size() );
+#endif
 
         m_list.sort();
+    }
 
     // The result of this function can be a blend of successes and failures, whose
     // mix is given by the Count()s of the two lists.  The return value indicates whether
@@ -338,57 +345,7 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* a
     // false definitely means failure.
 
     return retv;
-
-#else
-
-    bool retv = true;
-
-    for( unsigned ii = 0; ii < nicknames.size(); ii++ )
-    {
-        const wxString& nickname = nicknames[ii];
-
-        try
-        {
-            wxArrayString fpnames = aTable->FootprintEnumerate( nickname );
-
-            for( unsigned i=0;  i<fpnames.GetCount();  ++i )
-            {
-                std::auto_ptr<MODULE> 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
 
 

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_

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 );
 
@@ -930,6 +933,7 @@ void LEGACY_PLUGIN::loadSETUP()
 void LEGACY_PLUGIN::LoadMODULE( MODULE* aModule )
 {
     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<D_PAD> 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<DRAWSEGMENT>   dseg( new DRAWSEGMENT( m_board ) );
+
     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 )
                 {
@@ -1865,6 +1873,7 @@ void LEGACY_PLUGIN::loadPCB_TEXT()
     m_board->Add( pcbtxt, ADD_APPEND );
 
     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<DIMENSION> dim( new DIMENSION( m_board ) );
+
     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" ) )
             {