diff --git a/src/App/ComplexGeoData.cpp b/src/App/ComplexGeoData.cpp index f59bdc4329..4cb6994d6f 100644 --- a/src/App/ComplexGeoData.cpp +++ b/src/App/ComplexGeoData.cpp @@ -586,6 +586,11 @@ void ComplexGeoData::restoreStream(std::istream& stream, std::size_t count) // NOLINTNEXTLINE FC_THROWM(Base::RuntimeError, "Failed to restore element map " << _persistenceName); } + constexpr std::size_t oneGbOfInts {(1 << 30) / sizeof(int)}; + if (sCount > oneGbOfInts) { + // NOLINTNEXTLINE + FC_THROWM(Base::RuntimeError, "Failed to restore element map (>1GB) " << _persistenceName); + } sids.reserve(static_cast(sCount)); for (std::size_t j = 0; j < sCount; ++j) { long id = 0; diff --git a/src/App/ComplexGeoData.h b/src/App/ComplexGeoData.h index 8c0f8dc59b..366b2a1f6b 100644 --- a/src/App/ComplexGeoData.h +++ b/src/App/ComplexGeoData.h @@ -369,7 +369,7 @@ public: */ void traceElement(const MappedName& name, TraceCallback cb) const { - _elementMap->traceElement(name, Tag, cb); + _elementMap->traceElement(name, Tag, std::move(cb)); } /** Flush an internal buffering for element mapping */ diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 39abf869e7..e14c4c17ff 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -3809,6 +3809,9 @@ void Document::_removeObject(DocumentObject* pcObject) _checkTransaction(pcObject, nullptr, __LINE__); auto pos = d->objectMap.find(pcObject->getNameInDocument()); + if (pos == d->objectMap.end()) { + FC_ERR("Internal error, could not find " << pcObject->getFullName() << " to remove"); + } if (!d->rollback && d->activeUndoTransaction && pos->second->hasChildElement()) { // Preserve link group children global visibility. See comments in @@ -4279,15 +4282,15 @@ std::vector Document::getRootObjectsIgnoreLinks() const { std::vector ret; - for (auto objectIt : d->objectArray) { + for (const auto &objectIt : d->objectArray) { auto list = objectIt->getInList(); bool noParents = list.empty(); if (!noParents) { // App::Document getRootObjects returns the root objects of the dependency graph. - // So if an object is referenced by a App::Link, it will not be returned by that + // So if an object is referenced by an App::Link, it will not be returned by that // function. So here, as we want the tree-root level objects, we check if all the - // parents are links. In which case its still a root object. + // parents are links. In which case it's still a root object. noParents = std::all_of(list.cbegin(), list.cend(), [](App::DocumentObject* obj) { return obj->isDerivedFrom(); }); @@ -4307,21 +4310,19 @@ void DocumentP::findAllPathsAt(const std::vector& all_nodes, Path tmp) { if (std::ranges::find(tmp, id) != tmp.end()) { - Path tmp2(tmp); - tmp2.push_back(id); - all_paths.push_back(tmp2); + tmp.push_back(id); + all_paths.push_back(std::move(tmp)); return; // a cycle } tmp.push_back(id); if (all_nodes[id].empty()) { - all_paths.push_back(tmp); + all_paths.push_back(std::move(tmp)); return; } for (size_t i = 0; i < all_nodes[id].size(); i++) { - Path tmp2(tmp); - findAllPathsAt(all_nodes, all_nodes[id][i], all_paths, tmp2); + findAllPathsAt(all_nodes, all_nodes[id][i], all_paths, tmp); } } @@ -4349,20 +4350,19 @@ Document::getPathsByOutList(const App::DocumentObject* from, const App::Document size_t index_from = indexMap[from]; size_t index_to = indexMap[to]; - Path tmp; std::vector all_paths; - DocumentP::findAllPathsAt(all_nodes, index_from, all_paths, tmp); + DocumentP::findAllPathsAt(all_nodes, index_from, all_paths, Path()); for (const Path& it : all_paths) { auto jt = std::ranges::find(it, index_to); if (jt != it.end()) { - std::list path; + array.push_back({}); + auto& path = array.back(); for (auto kt = it.begin(); kt != jt; ++kt) { path.push_back(d->objectArray[*kt]); } path.push_back(d->objectArray[*jt]); - array.push_back(path); } } diff --git a/src/App/DocumentObject.cpp b/src/App/DocumentObject.cpp index 2b99aa7d7d..bc5d291e7b 100644 --- a/src/App/DocumentObject.cpp +++ b/src/App/DocumentObject.cpp @@ -1162,7 +1162,7 @@ void DocumentObject::Save(Base::Writer& writer) const void DocumentObject::setExpression(const ObjectIdentifier& path, std::shared_ptr expr) { - ExpressionEngine.setValue(path, expr); + ExpressionEngine.setValue(path, std::move(expr)); } /** diff --git a/src/App/DocumentObjectPyImp.cpp b/src/App/DocumentObjectPyImp.cpp index 5ca4a9185b..eedd8bea33 100644 --- a/src/App/DocumentObjectPyImp.cpp +++ b/src/App/DocumentObjectPyImp.cpp @@ -383,7 +383,7 @@ PyObject* DocumentObjectPy::setExpression(PyObject* args) shared_expr->comment = comment; } - getDocumentObjectPtr()->setExpression(p, shared_expr); + getDocumentObjectPtr()->setExpression(p, std::move(shared_expr)); Py_Return; } diff --git a/src/App/ElementMap.cpp b/src/App/ElementMap.cpp index 9e69c434be..88cdcde128 100644 --- a/src/App/ElementMap.cpp +++ b/src/App/ElementMap.cpp @@ -258,7 +258,8 @@ ElementMapPtr ElementMap::restore(::App::StringHasherRef hasherRef, std::istream std::vector childMaps; count = 0; - if (!(stream >> tmp >> count) || tmp != "MapCount" || count == 0) { + constexpr int practicalMaximum {1 << 30 / sizeof(ElementMapPtr)}; // a 1GB child map vector: almost certainly a bug + if (!(stream >> tmp >> count) || tmp != "MapCount" || count == 0 || count > practicalMaximum) { FC_THROWM(Base::RuntimeError, msg); // NOLINT } childMaps.reserve(count - 1); @@ -285,6 +286,10 @@ ElementMapPtr ElementMap::restore(::App::StringHasherRef hasherRef, if (!(stream >> tmp >> index >> id >> typeCount) || tmp != "ElementMap") { FC_THROWM(Base::RuntimeError, msg); // NOLINT } + constexpr int maxTypeCount(1000); + if (typeCount < 0 || typeCount > maxTypeCount) { + FC_THROWM(Base::RuntimeError, "Bad type count in element map, ignoring map"); // NOLINT + } auto& map = _idToElementMap[id]; if (map) { diff --git a/src/App/Expression.cpp b/src/App/Expression.cpp index 2750c9802a..115db3dd9b 100644 --- a/src/App/Expression.cpp +++ b/src/App/Expression.cpp @@ -1728,7 +1728,7 @@ FunctionExpression::FunctionExpression(const DocumentObject *_owner, Function _f : UnitExpression(_owner) , f(_f) , fname(std::move(name)) - , args(_args) + , args(std::move(_args)) { switch (f) { case ABS: @@ -2643,7 +2643,7 @@ Expression *FunctionExpression::simplify() const return eval(); } else - return new FunctionExpression(owner, f, std::string(fname), a); + return new FunctionExpression(owner, f, std::string(fname), std::move(a)); } /** @@ -2811,7 +2811,7 @@ Expression *FunctionExpression::_copy() const a.push_back((*i)->copy()); ++i; } - return new FunctionExpression(owner, f, std::string(fname), a); + return new FunctionExpression(owner, f, std::string(fname), std::move(a)); } void FunctionExpression::_visit(ExpressionVisitor &v) diff --git a/src/App/ExpressionTokenizer.cpp b/src/App/ExpressionTokenizer.cpp index aefa56c61f..751b860a36 100644 --- a/src/App/ExpressionTokenizer.cpp +++ b/src/App/ExpressionTokenizer.cpp @@ -75,14 +75,17 @@ QString ExpressionTokenizer::perform(const QString& prefix, int pos) // Pop those trailing tokens depending on the given position, which may be // in the middle of a token, and we shall include that token. for (auto it = tokens.begin(); it != tokens.end(); ++it) { - if (std::get<1>(*it) >= pos) { + int tokenType = std::get<0>(*it); + int location = std::get<1>(*it); + int tokenLength = static_cast (std::get<2>(*it).size()); + if (location >= pos) { // Include the immediately followed '.' or '#', because we'll be // inserting these separators too, in ExpressionCompleteModel::pathFromIndex() - if (it != tokens.begin() && std::get<0>(*it) != '.' && std::get<0>(*it) != '#') { + if (it != tokens.begin() && tokenType != '.' && tokenType != '#') { it = it - 1; } - tokens.resize(it - tokens.begin() + 1); - prefixEnd = start + std::get<1>(*it) + (int)std::get<2>(*it).size(); + tokens.resize(it - tokens.begin() + 1); // Invalidates it, but we already calculated tokenLength + prefixEnd = start + location + tokenLength; break; } } diff --git a/src/App/GeoFeature.cpp b/src/App/GeoFeature.cpp index 0f7175e3c6..886eb2f26f 100644 --- a/src/App/GeoFeature.cpp +++ b/src/App/GeoFeature.cpp @@ -223,14 +223,6 @@ bool GeoFeature::getCameraAlignmentDirection(Base::Vector3d& direction, const ch bool GeoFeature::hasMissingElement(const char* subname) { return Data::hasMissingElement(subname); - if (!subname) { - return false; - } - auto dot = strrchr(subname, '.'); - if (!dot) { - return subname[0] == '?'; - } - return dot[1] == '?'; } void GeoFeature::updateElementReference() diff --git a/src/App/GeoFeatureGroupExtension.cpp b/src/App/GeoFeatureGroupExtension.cpp index e1f1400dcb..be3379784b 100644 --- a/src/App/GeoFeatureGroupExtension.cpp +++ b/src/App/GeoFeatureGroupExtension.cpp @@ -421,10 +421,8 @@ bool GeoFeatureGroupExtension::extensionGetSubObject(DocumentObject*& ret, } } if (ret) { - if (dot) { - ++dot; - } - if (dot && *dot && !ret->hasExtension(App::LinkBaseExtension::getExtensionClassTypeId()) + ++dot; + if (*dot && !ret->hasExtension(App::LinkBaseExtension::getExtensionClassTypeId()) && !ret->hasExtension(App::GeoFeatureGroupExtension::getExtensionClassTypeId())) { // Consider this // Body @@ -451,7 +449,7 @@ bool GeoFeatureGroupExtension::extensionGetSubObject(DocumentObject*& ret, *mat *= const_cast(this)->placement().getValue().toMatrix(); } - ret = ret->getSubObject(dot ? dot : "", pyObj, mat, true, depth + 1); + ret = ret->getSubObject(dot, pyObj, mat, true, depth + 1); } } return true; diff --git a/src/Gui/DAGView/DAGRectItem.cpp b/src/Gui/DAGView/DAGRectItem.cpp index 3247e3c0ff..3a82758bc5 100644 --- a/src/Gui/DAGView/DAGRectItem.cpp +++ b/src/Gui/DAGView/DAGRectItem.cpp @@ -64,7 +64,7 @@ void RectItem::paint(QPainter* painter, const QStyleOptionGraphicsItem* option, styleOption.state |= QStyle::State_Selected; QPalette palette = styleOption.palette; QColor tempColor = palette.color(QPalette::Active, QPalette::Highlight); - tempColor.setAlphaF(0.15); + tempColor.setAlphaF(0.15F); palette.setColor(QPalette::Inactive, QPalette::Highlight, tempColor); styleOption.palette = palette; }