From dabcb2e506a323759fd76066b81ca458829abe42 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Thu, 20 Feb 2025 21:17:55 -0600 Subject: [PATCH 1/4] Spreadsheet: Fixes for coverity defects Feb 2025 --- src/Mod/Spreadsheet/App/Cell.cpp | 2 +- src/Mod/Spreadsheet/App/DisplayUnit.h | 2 +- src/Mod/Spreadsheet/App/PropertySheet.cpp | 4 ++-- src/Mod/Spreadsheet/App/Sheet.cpp | 10 +++++++++- src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp | 2 +- src/Mod/Spreadsheet/Gui/ZoomableView.h | 4 ++-- 6 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/Mod/Spreadsheet/App/Cell.cpp b/src/Mod/Spreadsheet/App/Cell.cpp index a5a1ee2c11..d6695a3978 100644 --- a/src/Mod/Spreadsheet/App/Cell.cpp +++ b/src/Mod/Spreadsheet/App/Cell.cpp @@ -543,7 +543,7 @@ void Cell::setDisplayUnit(const std::string& unit) if (newDisplayUnit != displayUnit) { PropertySheet::AtomicPropertyChange signaller(*owner); - displayUnit = newDisplayUnit; + displayUnit = std::move(newDisplayUnit); setUsed(DISPLAY_UNIT_SET, !displayUnit.isEmpty()); setDirty(); diff --git a/src/Mod/Spreadsheet/App/DisplayUnit.h b/src/Mod/Spreadsheet/App/DisplayUnit.h index a5ec4011f6..d698f93deb 100644 --- a/src/Mod/Spreadsheet/App/DisplayUnit.h +++ b/src/Mod/Spreadsheet/App/DisplayUnit.h @@ -39,7 +39,7 @@ public: explicit DisplayUnit(const std::string _stringRep = "", const Base::Unit _unit = Base::Unit(), double _scaler = 0.0) - : stringRep(_stringRep) + : stringRep(std::move(_stringRep)) , unit(_unit) , scaler(_scaler) {} diff --git a/src/Mod/Spreadsheet/App/PropertySheet.cpp b/src/Mod/Spreadsheet/App/PropertySheet.cpp index 221a109788..7643c6dfd6 100644 --- a/src/Mod/Spreadsheet/App/PropertySheet.cpp +++ b/src/Mod/Spreadsheet/App/PropertySheet.cpp @@ -787,7 +787,7 @@ void PropertySheet::setAlias(CellAddress address, const std::string& alias) App::ObjectIdentifier key(owner, oldAlias); App::ObjectIdentifier value(owner, alias.empty() ? address.toString() : alias); - m[key] = value; + m[key] = std::move(value); owner->getDocument()->renameObjectIdentifiers(m); } @@ -1397,7 +1397,7 @@ void PropertySheet::addDependencies(CellAddress key) // Insert into maps propertyNameToCellMap[propName].insert(key); - cellToPropertyNameMap[key].insert(propName); + cellToPropertyNameMap[key].insert(std::move(propName)); } } } diff --git a/src/Mod/Spreadsheet/App/Sheet.cpp b/src/Mod/Spreadsheet/App/Sheet.cpp index f94a1d9642..b810357488 100644 --- a/src/Mod/Spreadsheet/App/Sheet.cpp +++ b/src/Mod/Spreadsheet/App/Sheet.cpp @@ -104,7 +104,15 @@ Sheet::Sheet() Sheet::~Sheet() { - clearAll(); + try { + clearAll(); + } + catch (boost::wrapexcept&) { + // Don't let an exception propagate out of a destructor (calls terminate()) + Base::Console().Error( + "clearAll() resulted in an exception when deleting the spreadsheet : %s\n", + *pcNameInDocument); + } } /** diff --git a/src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp b/src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp index 6e9463abd4..6df7a3a10c 100644 --- a/src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp +++ b/src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp @@ -191,7 +191,7 @@ void DlgBindSheet::accept() addr = std::string("<<") + copy + ">>"; } else { - addr = copy; + addr = std::move(copy); } }; diff --git a/src/Mod/Spreadsheet/Gui/ZoomableView.h b/src/Mod/Spreadsheet/Gui/ZoomableView.h index a95e34e156..886d2b23ec 100644 --- a/src/Mod/Spreadsheet/Gui/ZoomableView.h +++ b/src/Mod/Spreadsheet/Gui/ZoomableView.h @@ -68,9 +68,9 @@ private: QPointer stv; QGraphicsScene m_scene; - QGraphicsProxyWidget* qpw; + QGraphicsProxyWidget* qpw {nullptr}; - int m_zoomLevel; + int m_zoomLevel {100}; protected: void focusOutEvent(QFocusEvent* event) override; From 9c097eba60dc945be9fc94b28aedb29577de81d8 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Thu, 20 Feb 2025 22:00:40 -0600 Subject: [PATCH 2/4] App: Small fixes caught by Coverity scan --- src/App/Application.cpp | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/App/Application.cpp b/src/App/Application.cpp index 90b69881cc..b91911c039 100644 --- a/src/App/Application.cpp +++ b/src/App/Application.cpp @@ -1360,7 +1360,7 @@ void Application::addImportType(const char* Type, const char* ModuleName) std::string::size_type next = item.filter.find_first_of(" )", pos+1); std::string::size_type len = next-pos-2; std::string type = item.filter.substr(pos+2,len); - item.types.push_back(type); + item.types.push_back(std::move(type)); pos = item.filter.find("*.", next); } @@ -1368,12 +1368,12 @@ void Application::addImportType(const char* Type, const char* ModuleName) if (strncmp(Type, "FreeCAD", 7) == 0) { std::string AppName = Config()["ExeName"]; AppName += item.filter.substr(7); - item.filter = AppName; + item.filter = std::move(AppName); // put to the front of the array - _mImportTypes.insert(_mImportTypes.begin(),item); + _mImportTypes.insert(_mImportTypes.begin(),std::move(item)); } else { - _mImportTypes.push_back(item); + _mImportTypes.push_back(std::move(item)); } } @@ -1483,7 +1483,7 @@ void Application::addExportType(const char* Type, const char* ModuleName) std::string::size_type next = item.filter.find_first_of(" )", pos+1); std::string::size_type len = next-pos-2; std::string type = item.filter.substr(pos+2,len); - item.types.push_back(type); + item.types.push_back(std::move(type)); pos = item.filter.find("*.", next); } @@ -1491,12 +1491,12 @@ void Application::addExportType(const char* Type, const char* ModuleName) if (strncmp(Type, "FreeCAD", 7) == 0) { std::string AppName = Config()["ExeName"]; AppName += item.filter.substr(7); - item.filter = AppName; + item.filter = std::move(AppName); // put to the front of the array - _mExportTypes.insert(_mExportTypes.begin(),item); + _mExportTypes.insert(_mExportTypes.begin(),std::move(item)); } else { - _mExportTypes.push_back(item); + _mExportTypes.push_back(std::move(item)); } } @@ -1665,7 +1665,7 @@ void Application::slotBeforeRecompute(const Document& doc) void Application::slotOpenTransaction(const Document& d, string s) { - this->signalOpenTransaction(d, s); + this->signalOpenTransaction(d, std::move(s)); } void Application::slotCommitTransaction(const Document& d) @@ -1743,7 +1743,7 @@ void Application::destruct() // now save all other parameter files auto& paramMgr = _pcSingleton->mpcPramManager; - for (auto it : paramMgr) { + for (const auto &it : paramMgr) { if ((it.second != _pcSysParamMngr) && (it.second != _pcUserParamMngr)) { if (it.second->HasSerializer() && !it.second->IgnoreSave()) { Base::Console().Log("Saving %s...\n", it.first.c_str()); @@ -2449,7 +2449,7 @@ void processProgramOptions(const variables_map& vm, std::map(); - mConfig["SaveFile"] = file; + mConfig["SaveFile"] = vm["output"].as(); } if (vm.count("hidden")) { @@ -2519,7 +2518,7 @@ void processProgramOptions(const variables_map& vm, std::map Application::getCmdLineFiles() // getting file name std::ostringstream temp; temp << "OpenFile" << i; - - std::string file(mConfig[temp.str()]); - files.push_back(file); + files.emplace_back(mConfig[temp.str()]); } return files; @@ -3400,7 +3397,7 @@ void Application::ExtractUserPath() // Set the default macro directory // - std::vector macrodirs = subdirs; + std::vector macrodirs = std::move(subdirs); // Last use in this method, just move macrodirs.emplace_back("Macro"); std::filesystem::path macro = findPath(dataHome, customData, macrodirs, true); mConfig["UserMacroPath"] = Base::FileInfo::pathToString(macro) + PATHSEP; From 318d6e15551532119b127b7bd6cf21ed9c67fb9c Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Thu, 20 Feb 2025 22:01:31 -0600 Subject: [PATCH 3/4] Base: Stop exception from leaking from Console().* These are sometimes used in destructors, where a raised exception calls terminate() --- src/Base/Console.h | 12 +++++++++++- src/Mod/Spreadsheet/App/Sheet.cpp | 3 ++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Base/Console.h b/src/Base/Console.h index b2bd857e90..03f41d07ae 100644 --- a/src/Base/Console.h +++ b/src/Base/Console.h @@ -1169,7 +1169,17 @@ template(notifiername, format); diff --git a/src/Mod/Spreadsheet/App/Sheet.cpp b/src/Mod/Spreadsheet/App/Sheet.cpp index b810357488..55cf1b1ba5 100644 --- a/src/Mod/Spreadsheet/App/Sheet.cpp +++ b/src/Mod/Spreadsheet/App/Sheet.cpp @@ -24,6 +24,7 @@ #ifndef _PreComp_ #include +#include #include #include #include @@ -107,7 +108,7 @@ Sheet::~Sheet() try { clearAll(); } - catch (boost::wrapexcept&) { + catch (boost::regex_error&) { // Don't let an exception propagate out of a destructor (calls terminate()) Base::Console().Error( "clearAll() resulted in an exception when deleting the spreadsheet : %s\n", From 49c2888cdef3d0ddab68b64e699bddb1c999cc47 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Sun, 23 Feb 2025 14:25:30 -0600 Subject: [PATCH 4/4] Base: change exception to (...) Co-authored-by: Benjamin Nauck --- src/Mod/Spreadsheet/App/Sheet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mod/Spreadsheet/App/Sheet.cpp b/src/Mod/Spreadsheet/App/Sheet.cpp index 55cf1b1ba5..918563df91 100644 --- a/src/Mod/Spreadsheet/App/Sheet.cpp +++ b/src/Mod/Spreadsheet/App/Sheet.cpp @@ -108,7 +108,7 @@ Sheet::~Sheet() try { clearAll(); } - catch (boost::regex_error&) { + catch (...) { // Don't let an exception propagate out of a destructor (calls terminate()) Base::Console().Error( "clearAll() resulted in an exception when deleting the spreadsheet : %s\n",