From ae4e372e221f2114e8c9b0ec5f11d9a426bda24d Mon Sep 17 00:00:00 2001 From: Frank Winklmeier Date: Fri, 10 Nov 2017 16:57:23 +0100 Subject: [PATCH 1/2] Remove unnecessary long/StatusCode conversions In preparation for a larger re-design of StatusCode clean up a few places where the code unnecessarily relies on an implicit long type-conversion. In detail: - ObjectVector::remove: Return the index of the deleted object as documented in the base class. Before it always returned 1 - RegistryEntry: Change return type of some methods from long to StautsCode - IODataManager: Get rid of the S_OK/S_ERROR enum alias as this relies on the actual type of the StatusCode enums - RootDataConnection: Change return type of setCompression from long to StatusCode --- GaudiHive/src/AlgResourcePool.cpp | 5 +++-- GaudiKernel/GaudiKernel/ObjectVector.h | 5 +++-- GaudiKernel/GaudiKernel/RegistryEntry.h | 8 +++---- GaudiKernel/src/Lib/RegistryEntry.cpp | 12 +++++----- GaudiUtils/src/component/IODataManager.cpp | 26 ++++++++++------------ RootCnv/RootCnv/RootDataConnection.h | 2 +- RootCnv/src/RootDataConnection.cpp | 6 ++--- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/GaudiHive/src/AlgResourcePool.cpp b/GaudiHive/src/AlgResourcePool.cpp index cfe216721a..8a875eb02a 100644 --- a/GaudiHive/src/AlgResourcePool.cpp +++ b/GaudiHive/src/AlgResourcePool.cpp @@ -93,9 +93,10 @@ StatusCode AlgResourcePool::acquireAlgorithm( const std::string& name, IAlgorith StatusCode sc; if ( blocking ) { itQueueIAlgPtr->second->pop( algo ); - sc = StatusCode::SUCCESS; } else { - sc = itQueueIAlgPtr->second->try_pop( algo ); + if ( !itQueueIAlgPtr->second->try_pop( algo ) ) { + sc = StatusCode::FAILURE; + } } // Note that reentrant algos are not consumed so we put them diff --git a/GaudiKernel/GaudiKernel/ObjectVector.h b/GaudiKernel/GaudiKernel/ObjectVector.h index 19530a8c74..ec2dc4a625 100644 --- a/GaudiKernel/GaudiKernel/ObjectVector.h +++ b/GaudiKernel/GaudiKernel/ObjectVector.h @@ -197,13 +197,14 @@ public: if ( i == end() ) { // Object cannot be released from the conatiner, // as it is not contained in it - return StatusCode::FAILURE; + return 0; } + long idx = std::distance( begin(), i ); // Set the back pointer to 0 to avoid repetitional searching // for the object in the container and deleting the object ( *i )->setParent( nullptr ); erase( i ); - return StatusCode::SUCCESS; + return idx; } /// Insert "value" before "position" diff --git a/GaudiKernel/GaudiKernel/RegistryEntry.h b/GaudiKernel/GaudiKernel/RegistryEntry.h index 5c26bd6a85..c01fb920b4 100644 --- a/GaudiKernel/GaudiKernel/RegistryEntry.h +++ b/GaudiKernel/GaudiKernel/RegistryEntry.h @@ -146,11 +146,11 @@ namespace DataSvcHelpers void setObject( DataObject* obj ); /// Add entry to data store - virtual long add( std::string name, DataObject* pObject, bool is_soft = false ); + virtual StatusCode add( std::string name, DataObject* pObject, bool is_soft = false ); /// Add entry to data store - virtual long add( std::string name, IOpaqueAddress* pAddress, bool is_soft = false ); + virtual StatusCode add( std::string name, IOpaqueAddress* pAddress, bool is_soft = false ); /// Remove an entry from the store - virtual long remove( boost::string_ref name ); + virtual StatusCode remove( boost::string_ref name ); /// Add object to the container virtual long add( IRegistry* obj ); /// Remove an object from the container @@ -158,7 +158,7 @@ namespace DataSvcHelpers /// Delete all contained elements virtual long deleteElements(); /// traverse data tree - virtual long traverseTree( IDataStoreAgent* pAgent, int level = 0 ); + virtual StatusCode traverseTree( IDataStoreAgent* pAgent, int level = 0 ); }; } #endif // GAUDIKERNEL_REGISTRYENTRY_H diff --git a/GaudiKernel/src/Lib/RegistryEntry.cpp b/GaudiKernel/src/Lib/RegistryEntry.cpp index b9076756c6..b69c0487f2 100644 --- a/GaudiKernel/src/Lib/RegistryEntry.cpp +++ b/GaudiKernel/src/Lib/RegistryEntry.cpp @@ -151,7 +151,7 @@ long DataSvcHelpers::RegistryEntry::remove( IRegistry* obj ) } /// Remove entry from data store -long DataSvcHelpers::RegistryEntry::remove( boost::string_ref nam ) +StatusCode DataSvcHelpers::RegistryEntry::remove( boost::string_ref nam ) { if ( nam.front() == SEPARATOR ) nam.remove_prefix( 1 ); auto i = std::find_if( m_store.begin(), m_store.end(), [&]( const auto& n ) @@ -203,7 +203,7 @@ long DataSvcHelpers::RegistryEntry::i_add( RegistryEntry* pEntry ) } /// Add entry to the current data store item -long DataSvcHelpers::RegistryEntry::add( std::string name, DataObject* pObject, bool is_soft ) +StatusCode DataSvcHelpers::RegistryEntry::add( std::string name, DataObject* pObject, bool is_soft ) { RegistryEntry* entry = i_create( std::move( name ) ); if ( !entry ) return StatusCode::FAILURE; @@ -213,7 +213,7 @@ long DataSvcHelpers::RegistryEntry::add( std::string name, DataObject* pObject, } /// Add entry to the current data store item -long DataSvcHelpers::RegistryEntry::add( std::string name, IOpaqueAddress* pAddress, bool is_soft ) +StatusCode DataSvcHelpers::RegistryEntry::add( std::string name, IOpaqueAddress* pAddress, bool is_soft ) { RegistryEntry* entry = i_create( std::move( name ) ); if ( !entry ) return StatusCode::FAILURE; @@ -293,15 +293,15 @@ DataSvcHelpers::RegistryEntry* DataSvcHelpers::RegistryEntry::i_find( const Data } // Traverse registry tree -long DataSvcHelpers::RegistryEntry::traverseTree( IDataStoreAgent* pAgent, int level ) +StatusCode DataSvcHelpers::RegistryEntry::traverseTree( IDataStoreAgent* pAgent, int level ) { bool go_down = pAgent->analyse( this, level ); - long status = StatusCode::SUCCESS; + StatusCode status; if ( go_down ) { for ( auto& i : m_store ) { try { RegistryEntry* entry = CAST_REGENTRY( RegistryEntry*, i ); - entry->traverseTree( pAgent, level + 1 ); + entry->traverseTree( pAgent, level + 1 ).ignore(); } catch ( ... ) { status = StatusCode::FAILURE; } diff --git a/GaudiUtils/src/component/IODataManager.cpp b/GaudiUtils/src/component/IODataManager.cpp index 0797d7b770..8e6990fed5 100644 --- a/GaudiUtils/src/component/IODataManager.cpp +++ b/GaudiUtils/src/component/IODataManager.cpp @@ -39,8 +39,6 @@ using namespace Gaudi; DECLARE_COMPONENT( IODataManager ) -enum { S_OK = StatusCode::SUCCESS, S_ERROR = StatusCode::FAILURE }; - static std::set s_badFiles; /// IService implementation: Db event selector override @@ -80,7 +78,7 @@ StatusCode IODataManager::error( CSTR msg, bool rethrow ) MsgStream log( msgSvc(), name() ); log << MSG::ERROR << "Error: " << msg << endmsg; if ( rethrow ) System::breakExecution(); - return S_ERROR; + return StatusCode::FAILURE; } /// Get connection by owner instance (0=ALL) @@ -116,13 +114,13 @@ StatusCode IODataManager::connectWrite( Connection* con, IoType mode, CSTR docty /// Read raw byte buffer from input stream StatusCode IODataManager::read( Connection* con, void* const data, size_t len ) { - return establishConnection( con ).isSuccess() ? con->read( data, len ) : S_ERROR; + return establishConnection( con ).isSuccess() ? con->read( data, len ) : StatusCode::FAILURE; } /// Write raw byte buffer to output stream StatusCode IODataManager::write( Connection* con, const void* data, int len ) { - return establishConnection( con ).isSuccess() ? con->write( data, len ) : S_ERROR; + return establishConnection( con ).isSuccess() ? con->write( data, len ) : StatusCode::FAILURE; } /// Seek on the file described by ioDesc. Arguments as in ::seek() @@ -166,12 +164,12 @@ StatusCode IODataManager::disconnect( Connection* con ) } return sc; } - return S_ERROR; + return StatusCode::FAILURE; } StatusCode IODataManager::reconnect( Entry* e ) { - StatusCode sc = S_ERROR; + StatusCode sc = StatusCode::FAILURE; if ( e && e->connection ) { switch ( e->ioType ) { case Connection::READ: @@ -183,7 +181,7 @@ StatusCode IODataManager::reconnect( Entry* e ) sc = e->connection->connectWrite( e->ioType ); break; default: - return S_ERROR; + return StatusCode::FAILURE; } if ( sc.isSuccess() && e->ioType == Connection::READ ) { std::vector to_retire; @@ -221,7 +219,7 @@ StatusCode IODataManager::establishConnection( Connection* con ) if ( con->isConnected() ) { con->resetAge(); - return S_OK; + return StatusCode::SUCCESS; } auto i = m_connectionMap.find( con->name() ); if ( i != m_connectionMap.end() ) { @@ -230,9 +228,9 @@ StatusCode IODataManager::establishConnection( Connection* con ) m_incSvc->fireIncident( Incident( con->name(), IncidentType::FailInputFile ) ); return error( "Severe logic bug: Twice identical connection object for DSN:" + con->name(), true ); } - if ( reconnect( i->second ).isSuccess() ) return S_OK; + if ( reconnect( i->second ).isSuccess() ) return StatusCode::SUCCESS; } - return S_ERROR; + return StatusCode::FAILURE; } StatusCode IODataManager::connectDataIO( int typ, IoType rw, CSTR dataset, CSTR technology, bool keep_open, @@ -303,7 +301,7 @@ StatusCode IODataManager::connectDataIO( int typ, IoType rw, CSTR dataset, CSTR << files.size() << " entries." << endmsg; return IDataConnection::BAD_DATA_CONNECTION; } - return S_ERROR; + return StatusCode::FAILURE; } std::string fid; auto j = m_fidMap.find( dsn ); @@ -360,7 +358,7 @@ StatusCode IODataManager::connectDataIO( int typ, IoType rw, CSTR dataset, CSTR } } m_connectionMap.emplace( fid, e ); // note: only if we disconnect does e get deleted?? - return S_OK; + return StatusCode::SUCCESS; } // Here we open the file! if ( !reconnect( ( *fi ).second ).isSuccess() ) { @@ -369,7 +367,7 @@ StatusCode IODataManager::connectDataIO( int typ, IoType rw, CSTR dataset, CSTR error( "connectDataIO> Cannot connect to database: PFN=" + dsn + " FID=" + fid, false ).ignore(); return IDataConnection::BAD_DATA_CONNECTION; } - return S_OK; + return StatusCode::SUCCESS; } sc = connectDataIO( FID, rw, fid, technology, keep_open, connection ); if ( !sc.isSuccess() && m_quarantine ) { diff --git a/RootCnv/RootCnv/RootDataConnection.h b/RootCnv/RootCnv/RootDataConnection.h index e3ed89a59a..fb0b6a4d19 100644 --- a/RootCnv/RootCnv/RootDataConnection.h +++ b/RootCnv/RootCnv/RootDataConnection.h @@ -68,7 +68,7 @@ namespace Gaudi RootConnectionSetup() = default; /// Set the global compression level - static long setCompression( const std::string& compression ); + static StatusCode setCompression( const std::string& compression ); /// Access to global compression level static int compression(); diff --git a/RootCnv/src/RootDataConnection.cpp b/RootCnv/src/RootDataConnection.cpp index fc4941d11f..d1ed1ae7e4 100644 --- a/RootCnv/src/RootDataConnection.cpp +++ b/RootCnv/src/RootDataConnection.cpp @@ -93,7 +93,7 @@ starCheck: } /// Set the global compression level -long RootConnectionSetup::setCompression( const std::string& compression ) +StatusCode RootConnectionSetup::setCompression( const std::string& compression ) { #if ROOT_VERSION_CODE >= ROOT_VERSION( 5, 33, 0 ) int res = 0, level = ROOT::CompressionSettings( ROOT::kLZMA, 6 ); @@ -228,7 +228,7 @@ StatusCode RootDataConnection::connectRead() sc = m_tool->readRefs(); sc.ignore(); #if ROOT_VERSION_CODE >= ROOT_VERSION( 5, 33, 0 ) - if ( sc.getCode() == ROOT_READ_ERROR ) { + if ( sc == ROOT_READ_ERROR ) { IIncidentSvc* inc = m_setup->incidentSvc(); if ( inc ) { inc->fireIncident( Incident( pfn(), IncidentType::CorruptedInputFile ) ); @@ -297,7 +297,7 @@ StatusCode RootDataConnection::connectWrite( IoType typ ) if ( makeTool() ) { StatusCode sc = m_tool->readRefs(); sc.ignore(); - if ( sc.getCode() == ROOT_READ_ERROR ) { + if ( sc == ROOT_READ_ERROR ) { #if ROOT_VERSION_CODE >= ROOT_VERSION( 5, 33, 0 ) IIncidentSvc* inc = m_setup->incidentSvc(); if ( inc ) { -- GitLab From ec3d2ff13234758308b23687184d42adce47931b Mon Sep 17 00:00:00 2001 From: Frank Winklmeier Date: Tue, 14 Nov 2017 15:28:36 +0100 Subject: [PATCH 2/2] Remove implicit StatusCode casts Remove a few more trivial implicit bool/long casts of StatusCode. No functional difference of the code. --- GaudiAlg/GaudiAlg/TupleObj.h | 2 +- GaudiAlg/src/lib/GaudiCommon.icpp | 2 +- GaudiHive/src/bin/concurrentRun.cpp | 2 +- GaudiKernel/GaudiKernel/Property.h | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/GaudiAlg/GaudiAlg/TupleObj.h b/GaudiAlg/GaudiAlg/TupleObj.h index 08f7d90e38..04a35d8c4b 100644 --- a/GaudiAlg/GaudiAlg/TupleObj.h +++ b/GaudiAlg/GaudiAlg/TupleObj.h @@ -793,7 +793,7 @@ namespace Tuples { auto scs = std::initializer_list{ this->column( std::get( tup ).first, Gaudi::invoke( std::get( tup ).second, value ) )...}; - auto is_ok = []( const StatusCode& sc ) -> bool { return sc; }; + auto is_ok = []( const StatusCode& sc ) -> bool { return sc.isSuccess(); }; auto i = std::find_if_not( begin( scs ), end( scs ), is_ok ); if ( i != end( scs ) ) { // avoid unchecked StatusCodes... diff --git a/GaudiAlg/src/lib/GaudiCommon.icpp b/GaudiAlg/src/lib/GaudiCommon.icpp index 142cc7ed27..50c46f52e6 100644 --- a/GaudiAlg/src/lib/GaudiCommon.icpp +++ b/GaudiAlg/src/lib/GaudiCommon.icpp @@ -458,7 +458,7 @@ StatusCode GaudiCommon::Print( const std::string& msg, const StatusCode s // test status code if ( st.isSuccess() ) { - } else if ( StatusCode::FAILURE != st.getCode() ) { + } else if ( StatusCode::FAILURE != st ) { str << " StatusCode=" << st.getCode(); } else { str << " StatusCode=FAILURE"; diff --git a/GaudiHive/src/bin/concurrentRun.cpp b/GaudiHive/src/bin/concurrentRun.cpp index a25b18b45c..293458d448 100644 --- a/GaudiHive/src/bin/concurrentRun.cpp +++ b/GaudiHive/src/bin/concurrentRun.cpp @@ -69,5 +69,5 @@ int main( int argc, char** argv ) propMgr->setProperty( "JobOptionsPath", fileName ); propMgr->setProperty( "JobOptionsPreAction", params.str() ); propMgr->setProperty( "JobOptionsPostAction", postAction ); // TODO: grep this from command line - return appUI->run(); + return appUI->run().getCode(); } diff --git a/GaudiKernel/GaudiKernel/Property.h b/GaudiKernel/GaudiKernel/Property.h index 7c5b114278..71f8113135 100644 --- a/GaudiKernel/GaudiKernel/Property.h +++ b/GaudiKernel/GaudiKernel/Property.h @@ -840,7 +840,7 @@ public: bool load( PropertyBase& destination ) const override { return destination.assign( *this ); } - bool assign( const PropertyBase& source ) override { return fromString( source.toString() ); } + bool assign( const PropertyBase& source ) override { return fromString( source.toString() ).isSuccess(); } std::string toString() const override; @@ -880,7 +880,7 @@ public: bool load( PropertyBase& destination ) const override { return destination.assign( *this ); } - bool assign( const PropertyBase& source ) override { return fromString( source.toString() ); } + bool assign( const PropertyBase& source ) override { return fromString( source.toString() ).isSuccess(); } std::string toString() const override; -- GitLab