From 48d95620247b55ed629848cb613ed06dc65762c1 Mon Sep 17 00:00:00 2001 From: Daniele Date: Thu, 10 Oct 2024 10:30:58 +0200 Subject: [PATCH 1/2] More helpful exception text for Registerbase (#1080) * tiny changes in RegisterBase * set up a clearer exception for RegisterBase (in ActionRegister) * set up a clearer exception for RegisterBase (in CLToolRegister) * added a few lines of documentation fro the exception --------- Co-authored-by: Daniele Rapetti <5535617+Iximiel@users.noreply.github.com> --- src/core/ActionRegister.cpp | 16 +++++++++++++- src/core/CLToolRegister.cpp | 5 ++++- src/core/RegisterBase.h | 43 ++++++++++++++++++++++++++++++++----- 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/core/ActionRegister.cpp b/src/core/ActionRegister.cpp index 9054effa76..616a1cafe8 100644 --- a/src/core/ActionRegister.cpp +++ b/src/core/ActionRegister.cpp @@ -20,6 +20,7 @@ along with plumed. If not, see . +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ */ #include "ActionRegister.h" +#include "ModuleMap.h" #include "Action.h" #include @@ -35,7 +36,7 @@ std::unique_ptr ActionRegister::create(const ActionOptions&ao) { return create(images,ao); } -std::unique_ptr ActionRegister::create(const std::vector & images,const ActionOptions&ao) { +std::unique_ptr ActionRegister::create(const std::vector & images,const ActionOptions&ao) try { if(ao.line.size()<1)return nullptr; auto content=get(images,ao.line[0]); @@ -45,6 +46,17 @@ std::unique_ptr ActionRegister::create(const std::vector & images auto fullPath=getFullPath(images,ao.line[0]); nao.setFullPath(fullPath); return content.create(nao); +} catch (PLMD::ExceptionRegisterError &e ) { + auto& actionName = e.getMissingKey(); + e <<"Action \"" << actionName << "\" is not known."; + if (getModuleMap().count(actionName)>0) { + e << "\nAn Action named \"" + < CLToolRegister::create(const CLToolOptions&ao) { return create(images,ao); } -std::unique_ptr CLToolRegister::create(const std::vector & images,const CLToolOptions&ao) { +std::unique_ptr CLToolRegister::create(const std::vector & images,const CLToolOptions&ao) try { if(ao.line.size()<1)return nullptr; auto & content=get(images,ao.line[0]); CLToolOptions nao( ao,content.keys ); return content.create(nao); +} catch (PLMD::ExceptionRegisterError &e ) { + auto& toolName = e.getMissingKey(); + throw e <<"CL tool \"" << toolName << "\" is not known."; } CLToolRegister::ID CLToolRegister::add(std::string key,creator_pointer cp,keywords_pointer kp) { diff --git a/src/core/RegisterBase.h b/src/core/RegisterBase.h index d08c3e4a4c..8f6e80124e 100644 --- a/src/core/RegisterBase.h +++ b/src/core/RegisterBase.h @@ -24,6 +24,7 @@ #include "tools/Exception.h" #include +#include #include #include #include @@ -95,6 +96,34 @@ class Register { friend std::ostream & operator<<(std::ostream &log,const Register ®); }; +/// Class representing an error in a register +class ExceptionRegisterError : + public Exception { + /// The missing key + std::string missingKey; +public: + using Exception::Exception; + /// Sets the missing key + /// \param key The missing key + /// \return This exception + /// + /// ExceptionRegisterError can be used as a builder pattern: + /// `throw ExceptionRegisterError().setMissingKey(key);` + /// + /// the key can be retrieved with ExceptionRegisterError::getMissingKey() + ExceptionRegisterError& setMissingKey (std::string_view key) { + missingKey=key; + return *this; + } + /// Returns the missing key + const std::string& getMissingKey() const {return missingKey;} + template + ExceptionRegisterError& operator<<(const T & x) { + *static_cast(this) < @@ -208,7 +237,9 @@ const Content & RegisterBase::get(const std::vector & images,con auto qualified_key=imageToString(*image) + ":" + key; if(m.count(qualified_key)>0) return m.find(qualified_key)->second->content; } - plumed_assert(m.count(key)>0); + if (m.count(key) == 0 ) { + throw ExceptionRegisterError().setMissingKey(key); + } return m.find(key)->second->content; } @@ -220,7 +251,9 @@ const std::string & RegisterBase::getFullPath(const std::vector auto qualified_key=imageToString(*image) + ":" + key; if(m.count(qualified_key)>0) return m.find(qualified_key)->second->fullPath; } - plumed_assert(m.count(key)>0); + if (m.count(key) == 0 ) { + throw ExceptionRegisterError().setMissingKey(key); + } return m.find(key)->second->fullPath; } @@ -228,7 +261,9 @@ template const Content & RegisterBase::get(const std::string & key) const { // lock map for reading std::shared_lock lock(mutex); - plumed_assert(m.count(key)>0); + if (m.count(key) == 0 ) { + throw ExceptionRegisterError().setMissingKey(key); + } return m.find(key)->second->content; } @@ -288,5 +323,3 @@ void RegisterBase::clearStaged() noexcept { } #endif - - From 0ddd460af652ced13f27d717ae0b88e474b16b07 Mon Sep 17 00:00:00 2001 From: Daniele Date: Thu, 10 Oct 2024 10:39:51 +0200 Subject: [PATCH 2/2] Plumed h for gromacs (#1105) * now the warnings from this test comes only from Plumed.h * change ncat to ncpt and looks like there are no more warnings * mimickying strdup to avoid a -Wstringop-overflow warning * trying with memcpy to avoid the security warning --- regtest/basic/rt-make-exceptions/main.cpp | 4 ++-- src/wrapper/Plumed.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/regtest/basic/rt-make-exceptions/main.cpp b/regtest/basic/rt-make-exceptions/main.cpp index baae4ae446..9d42429286 100644 --- a/regtest/basic/rt-make-exceptions/main.cpp +++ b/regtest/basic/rt-make-exceptions/main.cpp @@ -58,7 +58,7 @@ int main(){ int natoms=10; std::vector positions(3*natoms,0.0); - for(unsigned i=0;i<3*natoms;i++) positions[i]=i; + for(int i=0;i<3*natoms;i++) positions[i]=i; std::vector masses(natoms,1.0); std::vector forces(3*natoms,0.0); std::vector box(9,0.0); @@ -145,7 +145,7 @@ int main(){ plumed.cmd("setVirial",&virial[0]); plumed.cmd("setMasses",&masses[0]); // set positions after having passed the pointer. They should be accessed here (at "calc"). - for(unsigned i=0;i<3*natoms;i++) positions[i]=i*step; + for(int i=0;i<3*natoms;i++) positions[i]=i*step; plumed.cmd("calc"); // this should fail diff --git a/src/wrapper/Plumed.h b/src/wrapper/Plumed.h index 3ea2bc9c75..5094a301ad 100644 --- a/src/wrapper/Plumed.h +++ b/src/wrapper/Plumed.h @@ -2346,7 +2346,7 @@ class Plumed { char msg[__PLUMED_WRAPPER_CXX_EXCEPTION_BUFFER]; void init(const char* msg) __PLUMED_WRAPPER_CXX_NOEXCEPT { this->msg[0]='\0'; - __PLUMED_WRAPPER_STD strncat(this->msg,msg,__PLUMED_WRAPPER_CXX_EXCEPTION_BUFFER-1); + __PLUMED_WRAPPER_STD strncpy(this->msg,msg,__PLUMED_WRAPPER_CXX_EXCEPTION_BUFFER); this->msg[__PLUMED_WRAPPER_CXX_EXCEPTION_BUFFER-1]='\0'; if(PlumedGetenvExceptionsDebug() && __PLUMED_WRAPPER_STD strlen(msg) > __PLUMED_WRAPPER_CXX_EXCEPTION_BUFFER-1) __PLUMED_WRAPPER_STD fprintf(stderr,"+++ WARNING: message will be truncated\n"); } @@ -3831,7 +3831,7 @@ void* plumed_attempt_dlopen(const char*path,int mode) { __PLUMED_FPRINTF(stderr,"+++ Allocation error +++\n"); __PLUMED_WRAPPER_STD abort(); } - __PLUMED_WRAPPER_STD strncpy(pathcopy,path,strlenpath+1); + __PLUMED_WRAPPER_STD memcpy(pathcopy,path,strlenpath+1); pc=pathcopy+strlenpath-6; while(pc>=pathcopy && __PLUMED_WRAPPER_STD memcmp(pc,"Kernel",6)) pc--; if(pc>=pathcopy) {