haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix crash when duplicating symbol unit with alternate body style. 

* Add check for valid symbol library link in annotation code to prevent
  crash even if the symbol was not defined correctly.  This will assert
  in debug builds.
* Fix broken pin comparisons when library symbols have units with alternate
  body style defined.
* Remove some macros from LIB_SYMBOL comparison function which made
  debugging painful.
* Exit LIB_SYMBOL comparison function as soon as non-equivalent comparison
  occurs.
* Fix broken library symbol comparison due to name differences when
  checking for existing variants of the library symbol.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/14491
This commit is contained in:
Wayne Stambaugh 2023-04-08 15:28:45 -04:00
parent c6bcfda84c
commit d962b062ca
5 changed files with 60 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
eeschema/lib_pin.cpp
View File

@ -1416,6 +1416,23 @@ wxString LIB_PIN::GetItemDescription( UNITS_PROVIDER* aUnitsProvider ) const
}
std::ostream& LIB_PIN::operator<<( std::ostream& aStream )
{
aStream << "LIB_PIN:" << std::endl
<< " Name: \"" << m_name << "\"" << std::endl
<< " Number: \"" << m_number << "\"" << std::endl
<< " Position: " << m_position << std::endl
<< " Length: " << m_length << std::endl
<< " Orientation: " << PinOrientationName( m_orientation ) << std::endl
<< " Shape: " << PinShapeGetText( m_shape ) << std::endl
<< " Type: " << ElectricalPinTypeGetText( m_type ) << std::endl
<< " Name Text Size: " << m_nameTextSize << std::endl
<< " Number Text Size: " << m_numTextSize << std::endl;
return aStream;
}
#if defined(DEBUG)
void LIB_PIN::Show( int nestLevel, std::ostream& os ) const

6
eeschema/lib_pin.h
View File

@ -27,6 +27,7 @@
#ifndef CLASS_PIN_H
#define CLASS_PIN_H
#include <iostream>
#include <pin_type.h>
#include <lib_symbol.h>
@ -279,6 +280,11 @@ protected:
void printPinElectricalTypeName( const RENDER_SETTINGS* aSettings, VECTOR2I& aPosition,
int aOrientation, bool aDimmed );
bool operator==( const LIB_PIN& aRhs ) const { return compare( aRhs, EQUALITY ) == 0; }
bool operator<( const LIB_PIN& aRhs ) const { return compare( aRhs, EQUALITY ) < 0; }
bool operator>( const LIB_PIN& aRhs ) const { return compare( aRhs, EQUALITY ) > 0; }
std::ostream& operator<<( std::ostream& aStream );
private:
/**
* @copydoc LIB_ITEM::compare()

58
eeschema/lib_symbol.cpp
View File

@ -231,7 +231,6 @@ const LIB_SYMBOL& LIB_SYMBOL::operator=( const LIB_SYMBOL& aSymbol )
#define REPORT( msg ) { if( aReporter ) aReporter->Report( msg ); }
#define ITEM_DESC( item ) ( item )->GetItemDescription( &unitsProvider )
#define CHECKPOINT { if( retv && !aReporter ) return retv; }
int LIB_SYMBOL::Compare( const LIB_SYMBOL& aRhs, int aCompareFlags, REPORTER* aReporter ) const
{
@ -261,24 +260,22 @@ int LIB_SYMBOL::Compare( const LIB_SYMBOL& aRhs, int aCompareFlags, REPORTER* aR
{
retv = ( m_options == ENTRY_NORMAL ) ? -1 : 1;
REPORT( _( "Power flag differs." ) );
return retv;
}
CHECKPOINT;
if( int tmp = m_unitCount - aRhs.m_unitCount )
{
retv = tmp;
REPORT( _( "Unit count differs." ) );
return retv;
}
CHECKPOINT;
// Make sure shapes are sorted, but no need with fields and pins as we're going to
// match those up by id/name/number.
// Make sure shapes and pins are sorted. No need with fields as those are
// matched by id/name.
std::set<const LIB_ITEM*, LIB_ITEM::cmp_items> aShapes;
std::set<const LIB_ITEM*> aFields;
std::set<const LIB_ITEM*> aPins;
std::set<const LIB_ITEM*, LIB_ITEM::cmp_items> aPins;
for( auto it = m_drawings.begin(); it != m_drawings.end(); ++it )
{
@ -292,7 +289,7 @@ int LIB_SYMBOL::Compare( const LIB_SYMBOL& aRhs, int aCompareFlags, REPORTER* aR
std::set<const LIB_ITEM*, LIB_ITEM::cmp_items> bShapes;
std::set<const LIB_ITEM*> bFields;
std::set<const LIB_ITEM*> bPins;
std::set<const LIB_ITEM*, LIB_ITEM::cmp_items> bPins;
for( auto it = aRhs.m_drawings.begin(); it != aRhs.m_drawings.end(); ++it )
{
@ -308,6 +305,7 @@ int LIB_SYMBOL::Compare( const LIB_SYMBOL& aRhs, int aCompareFlags, REPORTER* aR
{
retv = tmp;
REPORT( _( "Graphic item count differs." ) );
return retv;
}
else
{
@ -317,16 +315,22 @@ int LIB_SYMBOL::Compare( const LIB_SYMBOL& aRhs, int aCompareFlags, REPORTER* aR
{
retv = tmp2;
REPORT( wxString::Format( _( "%s differs." ), ITEM_DESC( *aIt ) ) );
return retv;
}
}
}
CHECKPOINT;
if( int tmp = static_cast<int>( aPins.size() - bPins.size() ) )
{
retv = tmp;
REPORT( _( "Pin count differs." ) );
return retv;
}
for( const LIB_ITEM* aPinItem : aPins )
{
const LIB_PIN* aPin = static_cast<const LIB_PIN*>( aPinItem );
const LIB_PIN* bPin = aRhs.GetPin( aPin->GetNumber() );
const LIB_PIN* bPin = aRhs.GetPin( aPin->GetNumber(), aPin->GetUnit(), aPin->GetConvert() );
int tmp = 0;
if( !bPin )
@ -338,15 +342,10 @@ int LIB_SYMBOL::Compare( const LIB_SYMBOL& aRhs, int aCompareFlags, REPORTER* aR
{
retv = tmp;
REPORT( wxString::Format( _( "Pin %s differs." ), aPin->GetNumber() ) );
return retv;
}
}
if( int tmp = static_cast<int>( aPins.size() - bPins.size() ) )
{
retv = tmp;
REPORT( _( "Pin count differs." ) );
}
for( const LIB_ITEM* aFieldItem : aFields )
{
const LIB_FIELD* aField = static_cast<const LIB_FIELD*>( aFieldItem );
@ -381,23 +380,22 @@ int LIB_SYMBOL::Compare( const LIB_SYMBOL& aRhs, int aCompareFlags, REPORTER* aR
{
retv = tmp;
REPORT( wxString::Format( _( "%s field differs." ), aField->GetName( false ) ) );
return retv;
}
}
CHECKPOINT;
if( int tmp = static_cast<int>( aFields.size() - bFields.size() ) )
{
retv = tmp;
REPORT( _( "Field count differs." ) );
return retv;
}
CHECKPOINT;
if( int tmp = static_cast<int>( m_fpFilters.GetCount() - aRhs.m_fpFilters.GetCount() ) )
{
retv = tmp;
REPORT( _( "Footprint filters differs." ) );
return retv;
}
else
{
@ -407,65 +405,63 @@ int LIB_SYMBOL::Compare( const LIB_SYMBOL& aRhs, int aCompareFlags, REPORTER* aR
{
retv = tmp2;
REPORT( _( "Footprint filters differ." ) );
break;
return retv;
}
}
}
CHECKPOINT;
if( int tmp = m_description.Cmp( aRhs.m_description ) )
{
retv = tmp;
REPORT( _( "Symbol descriptions differ." ) );
return retv;
}
CHECKPOINT;
if( int tmp = m_keyWords.Cmp( aRhs.m_keyWords ) )
{
retv = tmp;
REPORT( _( "Symbol keywords differ." ) );
return retv;
}
CHECKPOINT;
if( int tmp = m_pinNameOffset - aRhs.m_pinNameOffset )
{
retv = tmp;
REPORT( _( "Symbol pin name offsets differ." ) );
return retv;
}
CHECKPOINT;
if( ( aCompareFlags & LIB_ITEM::COMPARE_FLAGS::ERC ) == 0 )
{
if( m_showPinNames != aRhs.m_showPinNames )
{
retv = ( m_showPinNames ) ? 1 : -1;
REPORT( _( "Show pin names settings differ." ) );
return retv;
}
if( m_showPinNumbers != aRhs.m_showPinNumbers )
{
retv = ( m_showPinNumbers ) ? 1 : -1;
REPORT( _( "Show pin numbers settings differ." ) );
return retv;
}
if( m_includeInBom != aRhs.m_includeInBom )
{
retv = ( m_includeInBom ) ? 1 : -1;
REPORT( _( "Exclude from bill of materials settings differ." ) );
return retv;
}
if( m_includeOnBoard != aRhs.m_includeOnBoard )
{
retv = ( m_includeOnBoard ) ? 1 : -1;
REPORT( _( "Exclude from board settings differ." ) );
return retv;
}
}
CHECKPOINT;
if( !aReporter )
{
if( m_unitsLocked != aRhs.m_unitsLocked )

2
eeschema/sch_reference_list.cpp
View File

@ -564,6 +564,8 @@ void SCH_REFERENCE_LIST::Annotate( bool aUseSheetNum, int aSheetIntervalId, int
if( aStartAtCurrent && ref_unit.m_numRef > 0 )
minRefId = ref_unit.m_numRef;
wxCHECK( ref_unit.GetLibPart(), /* void */ );
// Annotation of one part per package symbols (trivial case).
if( ref_unit.GetLibPart()->GetUnitCount() <= 1 )
{

8
eeschema/sch_screen.cpp
View File

@ -192,12 +192,20 @@ void SCH_SCREEN::Append( SCH_ITEM* aItem, bool aUpdateLibSymbol )
wxCHECK2( foundSymbol, continue );
wxString tmp = symbol->GetLibSymbolRef()->GetName();
// Temporarily update the new symbol library symbol name so it
// doesn't fail on the name comparison below.
symbol->GetLibSymbolRef()->SetName( foundSymbol->GetName() );
if( *foundSymbol == *symbol->GetLibSymbolRef() )
{
newName = libSymbolName;
symbol->GetLibSymbolRef()->SetName( tmp );
break;
}
symbol->GetLibSymbolRef()->SetName( tmp );
foundSymbol = nullptr;
}