fixes: null pointers and references

Source/test/tmModelTester/tmModelTester.cpp

@@ -80,7 +80,7 @@ void DoReadFile(tmTree* theTree, const string& filename)
   try {
     theTree->GetSelf(fin);
   }
-  catch(...) {
+  catch (...) {
     cout << "Unexpected exception reading file " << fullname << endl;
     exit(1);
   }
@@ -106,11 +106,11 @@ void DoScaleOptimization(char* name)
   try {
     theOptimizer->Optimize();
   }
-  catch (tmNLCO::EX_BAD_CONVERGENCE ex) {
+  catch (const tmNLCO::EX_BAD_CONVERGENCE& ex) {
     cout << "Scale optimization failed with result code " << 
       ex.GetReason() << endl;
   }
-  catch(tmScaleOptimizer::EX_BAD_SCALE) {
+  catch (const tmScaleOptimizer::EX_BAD_SCALE&) {
     cout << "Scale optimization failed with scale too small. " << endl;
   }
   stopTime = clock();
@@ -135,7 +135,7 @@ Read in a file and perform an edge optimization on it.
 template <class NLCO>
 void DoEdgeOptimization(char* name)
 {
-      if(!name)return;
+    if(!name)return;
   const string filename = name;
   tmTree* theTree = new tmTree();
   DoReadFile(theTree, filename);
@@ -150,26 +150,31 @@ void DoEdgeOptimization(char* name)
   try {
     theOptimizer->Optimize();
   }
-  catch (tmNLCO::EX_BAD_CONVERGENCE ex) {
+  catch (const tmNLCO::EX_BAD_CONVERGENCE& ex) {
     cout << "Edge optimization failed with result code " << 
       ex.GetReason() << endl;
+    stopTime = clock();
+  }
+
+  if (stopTime < 0) {
   stopTime = clock();
+  }
+
   ReportCalls((filename + " objective").c_str(), 
-    theOptimizer->GetNLCO()->GetObjective());
+  theOptimizer->GetNLCO()->GetObjective());
   ReportCalls((filename + " constraint").c_str(), 
-    theOptimizer->GetNLCO()->GetConstraints());
-  delete theOptimizer;
-  delete theNLCO;
+  theOptimizer->GetNLCO()->GetConstraints());
+
   cout << "Optimized over " << movingNodes.size() << " nodes and " <<
-    stretchyEdges.size() << " edges. " << endl;
+  stretchyEdges.size() << " edges. " << endl;
   cout << "Edge strain is " << stretchyEdges.front()->GetStrain() << endl;
-  }
-  if (stopTime < 0)
-    stopTime = clock();
   cout << "Feasibility after optimization is " << 
-    (theTree->IsFeasible() ? "true " : "false") << endl;
+  (theTree->IsFeasible() ? "true " : "false") << endl;
   cout << "Elapsed time = " << (stopTime - startTime) << " ticks" << endl;
   cout << endl;
+
+  delete theOptimizer;
+  delete theNLCO;
   delete theTree;
 }
 
@@ -195,7 +200,7 @@ void DoStrainOptimization(char* name)
   try {
     theOptimizer->Optimize();
   }
-  catch (tmNLCO::EX_BAD_CONVERGENCE ex) {
+  catch (const tmNLCO::EX_BAD_CONVERGENCE& ex) {
     cout << "Strain optimization failed with result code " << 
       ex.GetReason() << endl;
   }

Source/tmModel/tmPtrClasses/tmDpptrTarget.cpp

@@ -12,6 +12,7 @@ Copyright:    ©2003 Robert J. Lang. All Rights Reserved.
 #include "tmDpptrSrc.h"
 
 #include <algorithm>
+#include <iostream>
 
 using namespace std;
 
@@ -68,16 +69,19 @@ Notify any DpptrSrcs that point to me to clear their pointers to me.
 *****/
 tmDpptrTarget::~tmDpptrTarget()
 {
-  //  Note: a tmDpptrArray<T> can hold multiple references to the same object, in which
-  //  case mDpptrSrcs will hold multiple pointers to the same tmDpptrArray<T>.
-  vector<tmDpptrSrc*> theDpptrSrcs(mDpptrSrcs);
-  for (size_t i = 0; i < theDpptrSrcs.size(); ++i) {
-    tmDpptrSrc* theDpptrSrc = theDpptrSrcs[i];
-    theDpptrSrc->RemoveDpptrTarget(this);
+  try {
+    //  Note: a tmDpptrArray<T> can hold multiple references to the same object, in which
+    //  case mDpptrSrcs will hold multiple pointers to the same tmDpptrArray<T>.
+    vector<tmDpptrSrc*> theDpptrSrcs(mDpptrSrcs);
+    for (size_t i = 0; i < theDpptrSrcs.size(); ++i) {
+      tmDpptrSrc* theDpptrSrc = theDpptrSrcs[i];
+      theDpptrSrc->RemoveDpptrTarget(this);
+    }
+  } catch (...) {
+    std::cout << "Exception caught while cleaning up tmDpptrTarget" << std::endl;
   }
 }
 
-
 /*****
 void tmDpptrTarget::AddDpptrSrc(tmDpptrSrc* r)
 Add a pointer-to-me

Source/tmModel/tmSolvers/tmStubFinder.cpp

@@ -222,11 +222,11 @@ void tmStubFinder::TestOneCombo(tmArray<tmStubInfo>& sInfoList)
   try {
     SolveEqns(10, u, 1.0e-8, 1.0e-8);
   }
-  catch(EX_TOO_MANY_ITERATIONS) {
+  catch(const EX_TOO_MANY_ITERATIONS&) {
     // no solution, so go on to the next case
     return;
   }
-  catch(EX_SINGULAR_MATRIX) {
+  catch(const EX_SINGULAR_MATRIX&) {
     // no solution, so go on to the next case
     return;
   }

Source/tmModel/tmTreeClasses/tmFacetOwner.cpp

@@ -64,7 +64,10 @@ inside of the poly.
 *****/
 bool tmFacetOwner::CanStartFacetFwd(tmCrease* aCrease)
 {
-  if (aCrease->mFwdFacet) return false;
+  if (!aCrease || !aCrease->mVertices.front() || !aCrease->mVertices.back() || 
+      !FacetOwnerAsPoly() || aCrease->mFwdFacet) {
+    return false;
+  }
   if (!aCrease->IsAxialCrease()) return true;
   return AreCCW(aCrease->mVertices.front()->mLoc, 
     aCrease->mVertices.back()->mLoc, FacetOwnerAsPoly()->mCentroid);
@@ -77,7 +80,10 @@ CanStartFacetBkd(), we will only start facets on non-axial creases.
 *****/
 bool tmFacetOwner::CanStartFacetBkd(tmCrease* aCrease)
 {
-  if (aCrease->mBkdFacet) return false;
+  if (!aCrease || !aCrease->mVertices.front() || !aCrease->mVertices.back() || 
+      !FacetOwnerAsPoly() || aCrease->mBkdFacet) {
+    return false;
+  }
   if (!aCrease->IsAxialCrease()) return true;
   return AreCW(aCrease->mVertices.front()->mLoc, 
     aCrease->mVertices.back()->mLoc, FacetOwnerAsPoly()->mCentroid);
@@ -133,40 +139,50 @@ counterclockwise. Also return the tmVertex (nextVertex) at the opposite end of
 the crease. 
 *****/
 void tmFacetOwner::GetNextCreaseAndVertex(tmCrease* thisCrease, 
-  tmVertex* thisVertex,  tmCrease*& nextCrease, tmVertex*& nextVertex)
+  tmVertex* thisVertex, tmCrease*& nextCrease, tmVertex*& nextVertex)
 {
+  // Validate input pointers
+  if (!thisCrease || !thisVertex || !thisCrease->mVertices.front() || !thisCrease->mVertices.back()) {
+    nextCrease = nullptr;
+    nextVertex = nullptr;
+    return;
+  }
+
   // Get the angle of thisCrease.
   tmVertex* thatVertex = thisCrease->mVertices.front();
-  if (thatVertex == thisVertex) thatVertex = thisCrease->mVertices.back();
+  if (thatVertex == thisVertex) {
+    thatVertex = thisCrease->mVertices.back();
+  }
+
+  // Validate vertices
+  if (!thatVertex || !thisVertex) {
+    nextCrease = nullptr;
+    nextVertex = nullptr;
+    return;
+  }
+
   tmFloat thisAngle = Angle(thatVertex->mLoc - thisVertex->mLoc);
 
-  // delta is the increment in angle from thisCrease to nextCrease. Start with
-  // the largest possible value. Then go through each of the Creases emanating
-  // from thisVertex and compare its angle to the angle of thisCrease. We'll
-  // keep the smallest positive angle we find.
   tmFloat delta = TWO_PI;
   nextCrease = thisCrease;
-  nextVertex = 0;
+  nextVertex = nullptr;
+
   for (size_t i = 0; i < thisVertex->mCreases.size(); ++i) {
     tmCrease* thatCrease = thisVertex->mCreases[i];
-    if (thatCrease == thisCrease) continue;
+    if (!thatCrease || thatCrease == thisCrease) continue;
 
-    // Get the angle of thatCrease and the tmVertex at the other end of
-    // thatCrease (thatVertex).
+    // Get vertices and validate
     thatVertex = thatCrease->mVertices.front();
-    if (thatVertex == thisVertex) thatVertex = thatCrease->mVertices.back();
-    tmFloat nextAngle = Angle(thatVertex->mLoc - thisVertex->mLoc);
+    if (!thatVertex || thatVertex == thisVertex) {
+      thatVertex = thatCrease->mVertices.back();
+    }
+    if (!thatVertex) continue;
 
-      // Find the angular increment to the new crease. Constrain it to lie
-      // between 0 and 2 PI.
+    tmFloat nextAngle = Angle(thatVertex->mLoc - thisVertex->mLoc);
     tmFloat newDelta = thisAngle - nextAngle;
     while (newDelta < 0) newDelta += TWO_PI;
     while (newDelta >= TWO_PI) newDelta -= TWO_PI;
 
-    // If the angular increment is less than our current champion, we'll
-    // replace the current values with those of the new tmCrease. When we're
-    // done, we'll be left with the tmCrease that makes the smallest angle with
-    // the current tmCrease.
     if (newDelta < delta) {
       delta = newDelta;
       nextCrease = thatCrease;
@@ -180,6 +196,7 @@ void tmFacetOwner::GetNextCreaseAndVertex(tmCrease* thisCrease,
 }
 
 
+
 /*****
 Starting with a list of creases, construct a complete network of
 counterclockwise facets with their creases and vertices.

Source/tmModel/tmTreeClasses/tmPath.cpp

@@ -5,7 +5,7 @@ Purpose:      Implementation file for class tmPath
 Author:       Robert J. Lang
 Modified by:  
 Created:      2003-11-25
-Copyright:    ©2003 Robert J. Lang. All Rights Reserved.
+Copyright:    2003 Robert J. Lang. All Rights Reserved.
 *******************************************************************************/
 
 #include "tmPath.h"
@@ -417,20 +417,20 @@ vertex creation before we create any creases owned by the path.
 tmVertex* tmPath::MakeVertex(const tmPoint& p, tmNode* aTreeNode)
 {
   // Create the new vertex and insert it into the list at the appropriate spot.
-  tmPoint& p1 = mNodes.front()->mLoc;
-  tmPoint& p2 = mNodes.back()->mLoc;
+  const tmPoint& p1 = mNodes.front()->mLoc;
+  const tmPoint& p2 = mNodes.back()->mLoc;
   tmFloat dist_p = Mag(p - p1);
   tmFloat x = dist_p / Mag(p2 - p1);
   tmFloat elevation = (1 - x) * mNodes.front()->mElevation + x * mNodes.back()->mElevation;
 
-  // Create vertex and store it in a vector to track ownership
-  std::vector<tmVertex*> newVertices;
-  std::unique_ptr<tmVertex> theVertex = std::make_unique<tmVertex>(mTree, this, p, elevation, mIsBorderPath, aTreeNode);
-  newVertices.push_back(theVertex);
-
+  // Create vertex and transfer ownership to mOwnedVertices
+  auto vertex = std::make_unique<tmVertex>(mTree, this, p, elevation, mIsBorderPath, aTreeNode);
+  auto theVertex = vertex.get(); // Keep raw pointer for return value
+  mOwnedVertices.push_back(vertex.release()); // Transfer ownership to mOwnedVertices
+  
   // Insert vertex at correct position
   for (size_t i = 0; i < mOwnedVertices.size() - 1; ++i) {
-    tmVertex* testVertex = mOwnedVertices[i];
+    const tmVertex* testVertex = mOwnedVertices[i];
     tmFloat dist_t = Mag(testVertex->mLoc - p1);
     if (dist_p < dist_t) {
       mOwnedVertices.MoveItem(mOwnedVertices.size(), (i + 1));
@@ -439,8 +439,7 @@ tmVertex* tmPath::MakeVertex(const tmPoint& p, tmNode* aTreeNode)
   }
 
   // Handle crease splitting
-  for (size_t i = 0; i < mOwnedCreases.size(); ++i) {
-    tmCrease* theCrease = mOwnedCreases[i];
+  for (auto* theCrease : mOwnedCreases) {
     tmVertex* frontVertex = theCrease->mVertices.front();
     tmVertex* backVertex = theCrease->mVertices.back();
 
@@ -451,8 +450,12 @@ tmVertex* tmPath::MakeVertex(const tmPoint& p, tmNode* aTreeNode)
 
     if (x > 0 && x < 1) {
       TMLOG("tmPath::MakeVertex(..) -- crease split during vertex creation");
-      std::make_unique<tmCrease>(mTree, this, frontVertex, theVertex, theCrease->mKind);
+      // Create new creases and ensure they're properly owned
+      auto newCrease1 = std::make_unique<tmCrease>(mTree, this, frontVertex, theVertex, theCrease->mKind);
       auto newCrease2 = std::make_unique<tmCrease>(mTree, this, theVertex, backVertex, theCrease->mKind);
+      mOwnedCreases.push_back(newCrease1.release());
+      mOwnedCreases.push_back(newCrease2.release());
+      // Now that new creases are created and owned, we can safely delete the old one
       delete theCrease;
       break;
     }
@@ -461,6 +464,7 @@ tmVertex* tmPath::MakeVertex(const tmPoint& p, tmNode* aTreeNode)
   return theVertex;
 }
 
+
 /*****
 Return front vertex in the path, i.e., the vertex corresponding to the front
 node. Note: this is NOT the front vertex in mOwnedVertices. Return 0 if the
@@ -513,13 +517,15 @@ void tmPath::BuildSelfVertices()
     tmPath* maxOutsetPath;
     tmFloat maxFrontReduction, maxBackReduction;
     GetMaxOutsetPath(maxOutsetPath, maxFrontReduction, maxBackReduction);
+    // maxOutsetPath is a view into an existing path, no ownership transfer
     tmFloat curPos = -maxFrontReduction;
     // Step through the nodes and edges of the path. Only create a vertex if
     // the position falls within the path.
     TMASSERT(maxOutsetPath->mEdges.not_empty());
-    for (size_t i = 0; i < maxOutsetPath->mEdges.size(); ++i) {
-      tmNode* curNode = maxOutsetPath->mNodes[i + 1];
-      curPos += maxOutsetPath->mEdges[i]->GetStrainedScaledLength();
+    size_t nodeIndex = 1;
+    for (const auto& edge : maxOutsetPath->mEdges) {
+      tmNode* curNode = maxOutsetPath->mNodes[nodeIndex++];
+      curPos += edge->GetStrainedScaledLength();
       if (curPos <= 0.0) continue;
       if (curPos >= mActPaperLength) break;
       GetOrMakeVertex(q1 + qu * curPos, curNode);