lib File Corruption if Field Has Double Quote

[Stephen]

The .lib files, which contain parts used in the schematic editor, uses the double quote character (") as a delimiter to separate field value, field coordinates, and field label data. If, in the library editor, the fields are edited using the T icon at the top of the window, and a string containing a double quote character is copy and pasted into the value part of the field, the double quote character corrupts the lib file. It happens because a double quote character ends up in the wrong place for a delimiter.

After the editor opened by the T icon is closed by clicking on the OK button, the corruption manifests itself by Kicad crashing if the field is double clicked on. The only solution is to open the lib file in a word processor and editing out the quote character, or replacing it with two single quote (') characters. Kicad should do this replacement automatically to avoid this corruption or use a delimiter that cannot be used in the field.

This bug became apparent to me when I copy and pasted a string that had in the middle of it a dimension in units of inches and indicated inches with the double quote character. That particular field, Field 6, was being used as a description field.

This bug was found in Kicad installed from file KiCad-2010-05-05-BZR2356-final-WinXP_full_with_components_doc_autoinstall.exe in a Windows XP OS.

[Dick Hollenbeck (dickelbeck)]

Well described report.

But there are probably more such cases.

I don't mind fixing this one, but it would be nice to be able to get our
arms around all of them at once. This is the second such report in the last
two months.

The solution is to use EscapedUTF8().c_str() to format the field, and it
will "escape" the interior double quote with a backslash.

The counterpart to

std::string EscapedUTF8( const wxString& aString );

is

int ReadDelimitedText( char* aDest, const char* aSource, int aDestSize )

These two functions are a matched set, and reside in common/string.cpp. One
does the reverse of the other.

Installing them is not hard, but doing one per month is not fun.

I will fix this one, but plea for somebody to help me find all the cases
where this is going to be a problem moving forward.

I guess we can grep for

" \"%s\" "

which is probably a good way to find most of the problems NOW.

[Dick Hollenbeck (dickelbeck)]

The problem extends to, but is not limited to ALL these cases, this is probably about 85% of them:

$ grep --include="*.cpp" -r '\\"%s\\"' . | grep TO_UTF8
./eeschema/dialogs/dialog_build_BOM.cpp: fprintf( f, "%c\"%s\"", s_ExportSeparatorSymbol, TO_UTF8( refNames ) );
./eeschema/sch_screen.cpp: || fprintf( aFile, "Title \"%s\"\n", TO_UTF8( m_Title ) ) < 0
./eeschema/sch_screen.cpp: || fprintf( aFile, "Date \"%s\"\n", TO_UTF8( m_Date ) ) < 0
./eeschema/sch_screen.cpp: || fprintf( aFile, "Rev \"%s\"\n", TO_UTF8( m_Revision ) ) < 0
./eeschema/sch_screen.cpp: || fprintf( aFile, "Comp \"%s\"\n", TO_UTF8( m_Company ) ) < 0
./eeschema/sch_screen.cpp: || fprintf( aFile, "Comment1 \"%s\"\n", TO_UTF8( m_Commentaire1 ) ) < 0
./eeschema/sch_screen.cpp: || fprintf( aFile, "Comment2 \"%s\"\n", TO_UTF8( m_Commentaire2 ) ) < 0
./eeschema/sch_screen.cpp: || fprintf( aFile, "Comment3 \"%s\"\n", TO_UTF8( m_Commentaire3 ) ) < 0
./eeschema/sch_screen.cpp: || fprintf( aFile, "Comment4 \"%s\"\n", TO_UTF8( m_Commentaire4 ) ) < 0
./eeschema/sch_sheet.cpp: if( fprintf( aFile, "F0 \"%s\" %d\n", TO_UTF8( m_SheetName ),
./eeschema/sch_sheet.cpp: if( fprintf( aFile, "F1 \"%s\" %d\n", TO_UTF8( m_FileName ),
./eeschema/sch_field.cpp: if( fprintf( aFile, " \"%s\"", TO_UTF8( m_Name ) ) == EOF )
./eeschema/lib_field.cpp: && fprintf( ExportFile, " \"%s\"", TO_UTF8( m_name ) ) < 0 )
./eeschema/netform.cpp: fprintf( f, "\"%s\"\n", TO_UTF8( Title ) );
./eeschema/netform.cpp: fprintf( f, " \"%s\"", TO_UTF8( msg ) );
./pcbnew/class_netinfo_item.cpp: fprintf( aFile, "Na %d \"%s\"\n", GetNet(), TO_UTF8( m_Netname ) );
./pcbnew/class_netinfo_item.cpp: // fprintf( aFile, "NetClass \"%s\"\n", TO_UTF8(m_NetClassName) );
./pcbnew/class_netclass.cpp: fprintf( aFile, "Name \"%s\"\n", TO_UTF8( m_Name ) );
./pcbnew/class_netclass.cpp: fprintf( aFile, "Desc \"%s\"\n", TO_UTF8( GetDescription() ) );
./pcbnew/class_netclass.cpp: fprintf( aFile, "AddNet \"%s\"\n", TO_UTF8( *i ) );
./pcbnew/class_module.cpp: fprintf( File, "Na \"%s\"\n", TO_UTF8( t3D->m_Shape3DName ) );
./pcbnew/class_dimension.cpp: fprintf( aFile, "Te \"%s\"\n", TO_UTF8( m_Text->m_Text ) );
./pcbnew/ioascii.cpp: fprintf( File, "Title \"%s\"\n", TO_UTF8( screen->m_Title ) );
./pcbnew/ioascii.cpp: fprintf( File, "Date \"%s\"\n", TO_UTF8( screen->m_Date ) );
./pcbnew/ioascii.cpp: fprintf( File, "Rev \"%s\"\n", TO_UTF8( screen->m_Revision ) );
./pcbnew/ioascii.cpp: fprintf( File, "Comp \"%s\"\n", TO_UTF8( screen->m_Company ) );
./pcbnew/ioascii.cpp: fprintf( File, "Comment1 \"%s\"\n", TO_UTF8( screen->m_Commentaire1 ) );
./pcbnew/ioascii.cpp: fprintf( File, "Comment2 \"%s\"\n", TO_UTF8( screen->m_Commentaire2 ) );
./pcbnew/ioascii.cpp: fprintf( File, "Comment3 \"%s\"\n", TO_UTF8( screen->m_Commentaire3 ) );
./pcbnew/ioascii.cpp: fprintf( File, "Comment4 \"%s\"\n", TO_UTF8( screen->m_Commentaire4 ) );
./pcbnew/class_pad.cpp: fprintf( aFile, "Ne %d \"%s\"\n", GetNet(), TO_UTF8( m_Netname ) );
./pcbnew/gen_modules_placefile.cpp: sprintf( line, "footprint \"%s\"...

[Dick Hollenbeck (dickelbeck)]

> The counterpart to
>
> std::string EscapedUTF8( const wxString& aString );
>
> is
>
> int ReadDelimitedText( char* aDest, const char* aSource, int aDestSize );

int ReadDelimitedText( wxString* aDest, const char* aSource );

has just been added, which I just wrote now as an alternative to the C
string version (which requires you to guess the size of the recipient C
string, rather old school).

[Dick Hollenbeck (dickelbeck)]

Try this patch please.

[Dick Hollenbeck (dickelbeck)]

Jean Pierre,

Can you look over this patch and commit it if works for you?
Also, file class_text_mod.cpp, line 167 with patch applied has a @todo which needs some attention. This function looks broken to me, and if so, has been broken for a long time. I don't see that the 'm_text' is being read properly, the line offset seems wrong to me.

[Dick Hollenbeck (dickelbeck)]

=== modified file 'pcbnew/gen_modules_placefile.cpp'
--- pcbnew/gen_modules_placefile.cpp 2011-03-01 19:26:17 +0000
+++ pcbnew/gen_modules_placefile.cpp 2011-03-25 13:23:30 +0000
@@ -396,7 +396,7 @@
         sprintf( line, "value \"%s\"\n",
                  TO_UTF8( Module->m_Value->m_Text ) );

Missed another one, see above, i.e. value.

[Martin Errenst (imp-d)]

r2921 stable