realAscot/kicad-source
1
0
Fork 0
mirror of https://gitlab.com/kicad/code/kicad.git synced 2025-09-14 02:03:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

SYMBOL_LIBRARY_MANAGER: Clarify ownership semantics

Browse Source 
DeleteBuffer took a std::shared_ptr by value. This means
it claims to take (and keep) part-ownership of the pointed-to
item. However, this function does not take any ownership or
change the item, it just refers to it. It also does not null-check
the pointer. So it should be a const reference to the item.
Ownership semantics are then: the caller retains ownership
while the function executes.

(It also saves a shared_ptr ref count, but that won't be
important here).
This commit is contained in:
John Beard 2024-09-18 22:53:46 +01:00
parent 69889eff7c
commit a756b058ae
3 changed files with 19 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
eeschema/symbol_library_manager.cpp
View File

@ -590,7 +590,7 @@ bool SYMBOL_LIBRARY_MANAGER::RemoveSymbol( const wxString& aAlias, const wxStrin
bool retv = true;
retv &= libBuf.DeleteBuffer( symbolBuf );
retv &= libBuf.DeleteBuffer( *symbolBuf );
OnDataChanged();
return retv;
@ -964,16 +964,21 @@ bool LIB_BUFFER::UpdateBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf, LIB_SY
}
bool LIB_BUFFER::DeleteBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf )
bool LIB_BUFFER::DeleteBuffer( const SYMBOL_BUFFER& aSymbolBuf )
{
auto symbolBufIt = std::find( m_symbols.begin(), m_symbols.end(), aSymbolBuf );
const auto sameBufferPredicate = [&]( const std::shared_ptr<SYMBOL_BUFFER>& aBuf )
{
return aBuf.get() == &aSymbolBuf;
};
auto symbolBufIt = std::find_if( m_symbols.begin(), m_symbols.end(), sameBufferPredicate );
wxCHECK( symbolBufIt != m_symbols.end(), false );
bool retv = true;
// Remove all derived symbols to prevent broken inheritance.
if( HasDerivedSymbols( aSymbolBuf->GetSymbol()->GetName() )
&& ( removeChildSymbols( aSymbolBuf ) == 0 ) )
if( HasDerivedSymbols( aSymbolBuf.GetSymbol()->GetName() )
&& ( removeChildSymbols( aSymbolBuf ) == 0 ) )
{
retv = false;
}
@ -1219,15 +1224,13 @@ size_t LIB_BUFFER::GetDerivedSymbolNames( const wxString& aSymbolName, wxArraySt
}
int LIB_BUFFER::removeChildSymbols( std::shared_ptr<SYMBOL_BUFFER>& aSymbolBuf )
int LIB_BUFFER::removeChildSymbols( const SYMBOL_BUFFER& aSymbolBuf )
{
wxCHECK( aSymbolBuf, 0 );
int cnt = 0;
wxArrayString derivedSymbolNames;
std::deque<std::shared_ptr<SYMBOL_BUFFER>>::iterator it;
if( GetDerivedSymbolNames( aSymbolBuf->GetSymbol()->GetName(), derivedSymbolNames ) )
if( GetDerivedSymbolNames( aSymbolBuf.GetSymbol()->GetName(), derivedSymbolNames ) )
{
for( const wxString& symbolName : derivedSymbolNames )
{

7
eeschema/symbol_library_manager.h
View File

@ -110,7 +110,10 @@ public:
///< Update the buffered symbol with the contents of \a aCopy.
bool UpdateBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf, LIB_SYMBOL* aCopy );
bool DeleteBuffer( std::shared_ptr<SYMBOL_BUFFER> aSymbolBuf );
/**
* Delete the given symbol buffer from the library buffer.
*/
bool DeleteBuffer( const SYMBOL_BUFFER& aSymbolBuf );
void ClearDeletedBuffer() { m_deleted.clear(); }
@ -160,7 +163,7 @@ private:
* @param aParent is the #SYMBOL_BUFFER to check against.
* @return the count of #SYMBOL_BUFFER objects removed from the library.
*/
int removeChildSymbols( std::shared_ptr<SYMBOL_BUFFER>& aSymbolBuf );
int removeChildSymbols( const SYMBOL_BUFFER& aSymbolBuf );
private:
std::deque<std::shared_ptr<SYMBOL_BUFFER>> m_symbols;

3
qa/tests/eeschema/test_symbol_library_manager.cpp
View File

@ -133,7 +133,8 @@ BOOST_AUTO_TEST_CASE( LibBuffer )
BOOST_CHECK_EQUAL( libBuffer.GetHash(), 4 );
BOOST_CHECK( *libBuffer.GetSymbol( parentSymbol1->GetName() ) == *tmp );
libBuffer.DeleteBuffer( buf );
const bool deletedOk = libBuffer.DeleteBuffer( *buf );
BOOST_CHECK( deletedOk );
BOOST_CHECK( libBuffer.GetBuffers().empty() );
}