From afabaea5cb71b8078ce7d3210528171f4d5d9140 Mon Sep 17 00:00:00 2001 From: wmayer Date: Sun, 13 Mar 2022 12:10:01 +0100 Subject: [PATCH] PD: Fix several coverity issues: * CID 350622: Negative array index read * CID 350613: Negative array index read * CID 166163: Negative array index read * refactor Hole::updateDiameterParam() --- src/Mod/PartDesign/App/FeatureHole.cpp | 48 +++++++++++++++++++------- src/Mod/PartDesign/App/FeatureHole.h | 2 ++ 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/Mod/PartDesign/App/FeatureHole.cpp b/src/Mod/PartDesign/App/FeatureHole.cpp index d48070eab9..d807848723 100644 --- a/src/Mod/PartDesign/App/FeatureHole.cpp +++ b/src/Mod/PartDesign/App/FeatureHole.cpp @@ -979,6 +979,12 @@ double Hole::getThreadPitch() const { int threadType = ThreadType.getValue(); int threadSize = ThreadSize.getValue(); + if (threadType < 0) { + throw Base::IndexError("Thread type out of range"); + } + if (threadSize < 0) { + throw Base::IndexError("Thread size out of range"); + } return threadDescription[threadType][threadSize].pitch; } @@ -1021,7 +1027,7 @@ void Hole::updateThreadDepthParam() } } -void Hole::updateDiameterParam() +std::optional Hole::determineDiameter() const { // Diameter parameter depends on Threaded, ThreadType, ThreadSize, and ThreadFit @@ -1031,14 +1037,14 @@ void Hole::updateDiameterParam() // When restoring the feature it might be in an inconsistent state. // So, just silently ignore it instead of throwing an exception. if (isRestoring()) - return; + return std::nullopt; throw Base::IndexError("Thread type out of range"); } if (threadSize < 0) { // When restoring the feature it might be in an inconsistent state. // So, just silently ignore it instead of throwing an exception. if (isRestoring()) - return; + return std::nullopt; throw Base::IndexError("Thread size out of range"); } double diameter = threadDescription[threadType][threadSize].diameter; @@ -1046,7 +1052,7 @@ void Hole::updateDiameterParam() double clearance = 0.0; if (threadType == 0) - return; + return std::nullopt; if (Threaded.getValue()) { @@ -1058,10 +1064,10 @@ void Hole::updateDiameterParam() } // use normed diameters if possible - std::string threadType = ThreadType.getValueAsString(); - if (threadType == "ISOMetricProfile" || threadType == "UNC" - || threadType == "UNF" || threadType == "UNEF") { - diameter = threadDescription[ThreadType.getValue()][ThreadSize.getValue()].CoreHole + clearance; + std::string threadTypeStr = ThreadType.getValueAsString(); + if (threadTypeStr == "ISOMetricProfile" || threadTypeStr == "UNC" + || threadTypeStr == "UNF" || threadTypeStr == "UNEF") { + diameter = threadDescription[threadType][threadSize].CoreHole + clearance; } // if nothing available, we must calculate else { @@ -1071,9 +1077,9 @@ void Hole::updateDiameterParam() } else { // we have a clearance hole bool found = false; - std::string threadType = ThreadType.getValueAsString(); + std::string threadTypeStr = ThreadType.getValueAsString(); // UTS and metric have a different clearance hole set - if (threadType == "ISOMetricProfile" || threadType == "ISOMetricFineProfile") { + if (threadTypeStr == "ISOMetricProfile" || threadTypeStr == "ISOMetricFineProfile") { int MatrixRowSizeMetric = sizeof(metricHoleDiameters) / sizeof(metricHoleDiameters[0]); switch (ThreadFit.getValue()) { case 0: /* standard fit */ @@ -1123,7 +1129,7 @@ void Hole::updateDiameterParam() throw Base::IndexError("Thread fit out of range"); } } - else if (threadType == "UNC" || threadType == "UNF" || threadType == "UNEF") { + else if (threadTypeStr == "UNC" || threadTypeStr == "UNF" || threadTypeStr == "UNEF") { std::string ThreadSizeString = ThreadSize.getValueAsString(); int MatrixRowSizeUTS = sizeof(UTSHoleDiameters) / sizeof(UTSHoleDiameters[0]); switch (ThreadFit.getValue()) { @@ -1197,7 +1203,14 @@ void Hole::updateDiameterParam() } } } - Diameter.setValue(diameter); + + return std::optional{diameter}; +} + +void Hole::updateDiameterParam() +{ + if (auto opt = determineDiameter()) + Diameter.setValue(opt.value()); } void Hole::onChanged(const App::Property* prop) @@ -1983,13 +1996,22 @@ TopoDS_Compound Hole::findHoles(const TopoDS_Shape& profileshape, const TopoDS_S TopoDS_Shape Hole::makeThread(const gp_Vec& xDir, const gp_Vec& zDir, double length) { + int threadType = ThreadType.getValue(); + int threadSize = ThreadSize.getValue(); + if (threadType < 0) { + throw Base::IndexError("Thread type out of range"); + } + if (threadSize < 0) { + throw Base::IndexError("Thread size out of range"); + } + bool leftHanded = (bool)ThreadDirection.getValue(); // Nomenclature and formulae according to Figure 1 of ISO 68-1 // this is the same for all metric and UTS threads as stated here: // https://en.wikipedia.org/wiki/File:ISO_and_UTS_Thread_Dimensions.svg // Note that in the ISO standard, Dmaj is called D, which has been followed here. - double D = threadDescription[ThreadType.getValue()][ThreadSize.getValue()].diameter; // Major diameter + double D = threadDescription[threadType][threadSize].diameter; // Major diameter double P = getThreadPitch(); double H = sqrt(3) / 2 * P; // Height of fundamental triangle diff --git a/src/Mod/PartDesign/App/FeatureHole.h b/src/Mod/PartDesign/App/FeatureHole.h index 645a388ee9..3400470aab 100644 --- a/src/Mod/PartDesign/App/FeatureHole.h +++ b/src/Mod/PartDesign/App/FeatureHole.h @@ -24,6 +24,7 @@ #ifndef PARTDESIGN_Hole_H #define PARTDESIGN_Hole_H +#include #include #include "json_fwd.hpp" #include "FeatureSketchBased.h" @@ -212,6 +213,7 @@ private: bool isDynamicCounterbore(const std::string &thread, const std::string &holeCutType); bool isDynamicCountersink(const std::string &thread, const std::string &holeCutType); void updateHoleCutParams(); + std::optional determineDiameter() const; void updateDiameterParam(); void updateThreadDepthParam(); void readCutDefinitions();