kicad-developers team mailing list archive

[Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>]

Or maybe something like this patch instead, to not leak the row (sorry, I
mixed up LIB_TABLE::InsertRow() in common/lib_table_base.cpp with the
function of the same name in new/sch_lib_table.cpp; the latter uses an
auto_ptr&, the former does not).

Or maybe this code is still in flux, and the ownership will be changed/fixed
in other ways - in any case, just wanted to point out this, now that I saw
it. The main point is to not use row->GetNickName() after giving up
ownership of the pointer to InsertRow().

 - Kristian.

Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx> writes:

> I stumbled upon this in SYMBOL_LIB_TABLE::Parse():
>
>     if( !InsertRow( row.release() ) )
>     {
>         wxString msg = wxString::Format(
>                             _( "'%s' is a duplicate symbol library nickname" ),
>                             GetChars( row->GetNickName() ) );
>
> I got a segfault from this, because the error message accesses row after
> row.release() has given up ownership.

diff --git a/eeschema/symbol_lib_table.cpp b/eeschema/symbol_lib_table.cpp
index 5ed5565..3cf8e7c 100644
--- a/eeschema/symbol_lib_table.cpp
+++ b/eeschema/symbol_lib_table.cpp
@@ -185,11 +185,13 @@ void SYMBOL_LIB_TABLE::Parse( LIB_TABLE_LEXER* in )
         // use doReplace in InsertRow().  (However a fallBack table can have a
         // conflicting nickName and ours will supercede that one since in
         // FindLib() we search this table before any fall back.)
-        if( !InsertRow( row.release() ) )
+        LIB_TABLE_ROW* row_to_insert = row.release();
+        if( !InsertRow( row_to_insert ) )
         {
             wxString msg = wxString::Format(
                                 _( "'%s' is a duplicate symbol library nickname" ),
-                                GetChars( row->GetNickName() ) );
+                                GetChars( row_to_insert->GetNickName() ) );
+            delete row_to_insert;
             THROW_PARSE_ERROR( msg, in->CurSource(), in->CurLine(), lineNum, offset );
         }
     }