[PPL-devel] [GIT] ppl/ppl(pip): Fixed memory managment issue in copy constructor of PIP_Problem.

[François Galea]

Module: ppl/ppl
Branch: pip
Commit: a4c21dff11473d99be2152f65e1d09a7a0319527
URL:    http://www.cs.unipr.it/git/gitweb.cgi?p=ppl/ppl.git;a=commit;h=a4c21dff11473d99be2152f65e1d09a7a0319527

Author: François Galea <francois.galea at uvsq.fr>
Date:   Fri Oct  9 13:08:37 2009 +0200

Fixed memory managment issue in copy constructor of PIP_Problem.

---

 src/PIP_Problem.cc               |    7 +++----
 src/PIP_Tree.cc                  |   31 +++++++++++++++++++++++++++++++
 src/PIP_Tree.defs.hh             |   12 ++++++++++++
 tests/PIP_Problem/pipproblem1.cc |   10 ++++++----
 4 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/src/PIP_Problem.cc b/src/PIP_Problem.cc
index 494d7f9..f2fb1c3 100644
--- a/src/PIP_Problem.cc
+++ b/src/PIP_Problem.cc
@@ -53,16 +53,15 @@ PPL::PIP_Problem::PIP_Problem(const PIP_Problem &y)
   : external_space_dim(y.external_space_dim),
     internal_space_dim(y.internal_space_dim),
     status(y.status),
-    // FIXME: this causes sharing of the solution tree,
-    // possibly later resulting in memory corruption (double free).
-    current_solution(y.current_solution),
+    current_solution(0),
     initialized(y.initialized),
     input_cs(y.input_cs),
     first_pending_constraint(y.first_pending_constraint),
     parameters(y.parameters),
     initial_context(y.initial_context) {
+  if (y.current_solution != 0)
+    current_solution = y.current_solution->clone();
   PPL_ASSERT(OK());
-  // FIXME: must also copy the solution tree
 }
 
 PPL::PIP_Problem::~PIP_Problem() {
diff --git a/src/PIP_Tree.cc b/src/PIP_Tree.cc
index b4c208e..f52221b 100644
--- a/src/PIP_Tree.cc
+++ b/src/PIP_Tree.cc
@@ -210,6 +210,16 @@ PIP_Decision_Node::PIP_Decision_Node(PIP_Tree_Node* fcp,
     false_child(fcp) {
 }
 
+PIP_Decision_Node ::PIP_Decision_Node(const PIP_Decision_Node& x)
+  : PIP_Tree_Node(x),
+    true_child(0),
+    false_child(0) {
+  if (x.true_child != 0)
+    true_child = x.true_child->clone();
+  if (x.false_child != 0)
+    false_child = x.false_child->clone();
+}
+
 const PIP_Solution_Node*
 PIP_Tree_Node::as_solution() const {
   return 0;
@@ -511,6 +521,27 @@ PIP_Tree_Node::ascii_load(std::istream& s) {
   return true;
 }
 
+PIP_Tree_Node*
+PIP_Tree_Node::clone() const {
+  const PIP_Solution_Node* as_s = as_solution();
+  if (as_s != 0)
+    return as_s->clone();
+  const PIP_Decision_Node* as_d = as_decision();
+  if (as_d != 0)
+    return as_d->clone();
+  return 0;
+}
+
+PIP_Solution_Node*
+PIP_Solution_Node::clone() const {
+  return new PIP_Solution_Node(*this);
+}
+
+PIP_Decision_Node*
+PIP_Decision_Node::clone() const {
+  return new PIP_Decision_Node(*this);
+}
+
 void
 PIP_Solution_Node::Tableau::ascii_dump(std::ostream& st) const {
   st << "denominator " << denominator << "\n";
diff --git a/src/PIP_Tree.defs.hh b/src/PIP_Tree.defs.hh
index e4a7e8b..4141818 100644
--- a/src/PIP_Tree.defs.hh
+++ b/src/PIP_Tree.defs.hh
@@ -120,6 +120,9 @@ public:
   void ascii_dump(std::ostream& s) const;
   bool ascii_load(std::istream& s);
 
+  //! Returns a pointer to a dynamically-allocated copy of \p *this.
+  PIP_Tree_Node* clone() const;
+
 protected:
   //! Default constructor.
   PIP_Tree_Node();
@@ -228,6 +231,9 @@ public:
   void ascii_dump(std::ostream& s) const;
   bool ascii_load(std::istream& s);
 
+  //! Returns a pointer to a dynamically-allocated copy of \p *this.
+  PIP_Solution_Node* clone() const;
+
   bool OK() const;
 
 private:
@@ -415,6 +421,9 @@ public:
   //! Returns a pointer to the \p v (true or false) branch of \p *this.
   PIP_Tree_Node* child_node(bool v);
 
+  //! Returns a pointer to a dynamically-allocated copy of \p *this.
+  PIP_Decision_Node* clone() const;
+
   bool OK() const;
 
 private:
@@ -446,6 +455,9 @@ private:
   PIP_Decision_Node(PIP_Tree_Node* fcp, PIP_Tree_Node* tcp);
 
 protected:
+  //! Copy constructor.
+  PIP_Decision_Node(const PIP_Decision_Node &x);
+
   /*! \brief
     Populates the parametric simplex tableau using external data, if necessary
 
diff --git a/tests/PIP_Problem/pipproblem1.cc b/tests/PIP_Problem/pipproblem1.cc
index 394a127..191727c 100644
--- a/tests/PIP_Problem/pipproblem1.cc
+++ b/tests/PIP_Problem/pipproblem1.cc
@@ -189,11 +189,14 @@ test04() {
                      cs.space_dimension());
   }
 
-  // Copy constructor is buggy.
+  // Copy constructor is no longer buggy.
   {
     PIP_Problem pip_copy = pip;
     // Here we call the destructor of pip_copy
-    // and we also destroy the (shared) solution tree of pip.
+    // and we also destroy the (copied) solution tree of pip_copy.
+    const PIP_Tree solution = pip_copy.solution();
+    display_solution(solution, params, Variables_Set(i, j),
+                     cs.space_dimension());
   }
 
   return ok;
@@ -205,6 +208,5 @@ BEGIN_MAIN
   DO_TEST(test01);
   DO_TEST(test02);
   DO_TEST(test03);
-  // Uncomment this to get a segmentation fault.
-  // DO_TEST(test04);
+  DO_TEST(test04);
 END_MAIN