Pcbnew: Fpid parser: Fix crash when reading a .kicad_pcb file containing a valid fpid with a revision value.

Fix an issue when a .kicad_pcb file contains an incorrect fpid ( containing a '/ ' in footprint section ) . After saving the board the file was no more readable, due to a broken fpid saved in file
(can happens in old .kicad_pcb files, coming from the bzr 4022 stable version, and/or in files converted from an other EDA tool like altium)
Fix 2 very minor coverity warnings.

common/fpid.cpp

@@ -29,6 +29,7 @@
 
 #include <macros.h>     // TO_UTF8()
 #include <fpid.h>
+#include <kicad_string.h>
 
 
 static inline bool isDigit( char c )
@@ -122,20 +123,17 @@ int FPID::Parse( const UTF8& aId )
 {
     clear();
 
-    size_t      cnt = aId.length() + 1;
-    char        tmp[cnt];  // C string for speed
-
-    std::strcpy( tmp, aId.c_str() );
-
-    const char* rev = EndsWithRev( tmp, tmp+aId.length(), '/' );
+    const char* buffer = aId.c_str();
+    const char* rev = EndsWithRev( buffer, buffer+aId.length(), '/' );
     size_t      revNdx;
     size_t      partNdx;
     int         offset;
 
     //=====<revision>=========================================
+    // in a FPID like discret:R3/rev4
     if( rev )
     {
-        revNdx = rev - aId.c_str();
+        revNdx = rev - buffer;
 
         // no need to check revision, EndsWithRev did that.
         revision = aId.substr( revNdx );
@@ -165,9 +163,14 @@ int FPID::Parse( const UTF8& aId )
 
     //=====<footprint name>====================================
     if( partNdx >= revNdx )
-        return partNdx;
+        return partNdx;     // Error: no footprint name.
 
-    SetFootprintName( aId.substr( partNdx, revNdx ) );
+    // Be sure the footprint name is valid.
+    // Some chars can be found in board file (in old board files
+    // or converted files from an other EDA tool
+    std::string fpname = aId.substr( partNdx, revNdx-partNdx );
+    ReplaceIllegalFileNameChars( &fpname, '_' );
+    SetFootprintName( UTF8( fpname ) );
 
     return -1;
 }
@@ -224,8 +227,8 @@ int FPID::SetFootprintName( const UTF8& aFootprintName )
 
     if( separation != -1 )
     {
-        nickname = aFootprintName.substr( separation+1 );
-        return separation + (int) nickname.size() + 1;
+        footprint = aFootprintName.substr( 0, separation-1 );
+        return separation;
     }
     else
     {

common/string.cpp

@@ -456,7 +456,7 @@ wxString GetIllegalFileNameWxChars()
 }
 
 
-bool ReplaceIllegalFileNameChars( std::string* aName )
+bool ReplaceIllegalFileNameChars( std::string* aName, int aReplaceChar )
 {
     bool              changed = false;
     std::string       result;
@@ -465,7 +465,11 @@ bool ReplaceIllegalFileNameChars( std::string* aName )
     {
         if( strchr( illegalFileNameChars, *it ) )
         {
-            StrPrintf( &result, "%%%02x", *it );
+            if( aReplaceChar )
+                StrPrintf( &result, "%c", aReplaceChar );
+            else
+                StrPrintf( &result, "%%%02x", *it );
+
             changed = true;
         }
         else

gerbview/rs274x.cpp

@@ -619,7 +619,7 @@ bool GERBER_IMAGE::ExecuteRS274XCommand( int       command,
     case AP_MACRO:  // lines like %AMMYMACRO*
                     // 5,1,8,0,0,1.08239X$1,22.5*
                     // %
-        ok = ReadApertureMacro( buff, text, m_Current_File );
+        /*ok = */ReadApertureMacro( buff, text, m_Current_File );
         break;
 
     case AP_DEFINITION:

include/fpid.h

@@ -82,6 +82,7 @@ public:
      */
     int Parse( const UTF8& aId );
 
+
     /**
      * Function GetLibNickname
      * returns the logical library name  portion of a FPID.
@@ -109,6 +110,9 @@ public:
     /**
      * Function SetFootprintName
      * overrides the footprint name portion of the FPID to @a aFootprintName
+     * @return int - minus 1 (i.e. -1) means success, >= 0 indicates the  character offset
+     *               into the parameter at which an error was detected, usually because it
+     *               contained '/'.
      */
     int SetFootprintName( const UTF8& aFootprintName );
 

include/kicad_string.h

@@ -158,11 +158,13 @@ wxString GetIllegalFileNameWxChars();
  * and are replaced with %xx where xx the hexadecimal equivalent of the replaced character.
  * This replacement may not be as elegant as using an underscore ('_') or hyphen ('-') but
  * it guarentees that there will be no naming conflicts when fixing footprint library names.
+ * however, if aReplaceChar is given, it will replace the illegal chars
  *
  * @param aName is a point to a std::string object containing the footprint name to verify.
+ * @param aReplaceChar (if not 0) is the replacement char.
  * @return true if any characters have been replaced in \a aName.
  */
-bool ReplaceIllegalFileNameChars( std::string* aName );
+bool ReplaceIllegalFileNameChars( std::string* aName, int aReplaceChar = 0 );
 
 #ifndef HAVE_STRTOKR
 // common/strtok_r.c optionally:

pcbnew/footprint_wizard_frame.cpp

@@ -420,6 +420,10 @@ void FOOTPRINT_WIZARD_FRAME::OnActivate( wxActivateEvent& event )
     if( !event.GetActive() )
         return;
 
+#if 0
+    // Currently, we do not have a way to see if a Python wizard has changed,
+    // therefore the lists of parameters and option has to be rebuilt
+    // This code could be enabled when this way exists
     bool footprintWizardsChanged = false;
 
     if( footprintWizardsChanged )
@@ -428,6 +432,7 @@ void FOOTPRINT_WIZARD_FRAME::OnActivate( wxActivateEvent& event )
         ReCreatePageList();
         DisplayWizardInfos();
     }
+#endif
 }