Make gmx_ana_pos_t a C++ class.

Add a constructor and destructor for it, and adjust memory management
where necessary.  Simplifies the code somewhat, and allows making
gmx_ana_indexmap_t a C++ class.

Change-Id: I7b6c9a3d8e14b35655980ef28b32cb82de281dc0

diff --git a/src/gromacs/selection/compiler.cpp b/src/gromacs/selection/compiler.cpp
index 60acf693f4f93c87b78f7cd10eee86ee66fabb51..1220c2c0ff73e9271038b3bcba5633cb1a883ef3 100644 (file)
--- a/src/gromacs/selection/compiler.cpp
+++ b/src/gromacs/selection/compiler.cpp
@@ -2582,7 +2582,7 @@ init_item_comg(const SelectionTreeElementPointer &sel,
                 gmx_ana_poscalc_set_flags(sel->u.expr.pc, cflags);
             }
             gmx_ana_poscalc_set_maxindex(sel->u.expr.pc, sel->cdata->gmax);
-            snew(sel->u.expr.pos, 1);
+            sel->u.expr.pos = new gmx_ana_pos_t();
             gmx_ana_poscalc_init_pos(sel->u.expr.pc, sel->u.expr.pos);
         }
     }

diff --git a/src/gromacs/selection/poscalc.cpp b/src/gromacs/selection/poscalc.cpp
index 6eedac6d08913674fbe9ae523b4a1d8ea03b18c5..4999669f129c54637f5008dd706aa2fd40664df3 100644 (file)
--- a/src/gromacs/selection/poscalc.cpp
+++ b/src/gromacs/selection/poscalc.cpp
@@ -770,7 +770,7 @@ create_simple_base(gmx_ana_poscalc_t *pc)
     base  = pc->coll->createCalculation(pc->type, flags);
     set_poscalc_maxindex(base, &pc->gmax, true);
 
-    snew(base->p, 1);
+    base->p = new gmx_ana_pos_t();
 
     pc->sbase = base;
     pc->coll->removeCalculation(base);
@@ -1086,10 +1086,7 @@ gmx_ana_poscalc_free(gmx_ana_poscalc_t *pc)
     {
         gmx_ana_index_deinit(&pc->gmax);
     }
-    if (pc->p)
-    {
-        gmx_ana_pos_free(pc->p);
-    }
+    delete pc->p;
     if (pc->sbase)
     {
         gmx_ana_poscalc_free(pc->sbase);

diff --git a/src/gromacs/selection/position.cpp b/src/gromacs/selection/position.cpp
index 4bdcf46e4cfcfb5c2320be67ba1fa7eba3b9c4a8..1713efc0268bacc8a67a87d46c053ae735567597 100644 (file)
--- a/src/gromacs/selection/position.cpp
+++ b/src/gromacs/selection/position.cpp
@@ -39,6 +39,8 @@
  * \author Teemu Murtola <teemu.murtola@gmail.com>
  * \ingroup module_selection
  */
+#include "gromacs/selection/position.h"
+
 #include <string.h>
 
 #include "gromacs/legacyheaders/smalloc.h"
@@ -46,22 +48,23 @@
 #include "gromacs/legacyheaders/vec.h"
 
 #include "gromacs/selection/indexutil.h"
-#include "gromacs/selection/position.h"
 #include "gromacs/utility/gmxassert.h"
 
-/*!
- * \param[out] pos      Output structure.
- *
- * Any contents of \p pos are discarded without freeing.
- */
-void
-gmx_ana_pos_clear(gmx_ana_pos_t *pos)
+gmx_ana_pos_t::gmx_ana_pos_t()
 {
-    pos->x  = NULL;
-    pos->v  = NULL;
-    pos->f  = NULL;
-    gmx_ana_indexmap_clear(&pos->m);
-    pos->nalloc_x = 0;
+    x = NULL;
+    v = NULL;
+    f = NULL;
+    gmx_ana_indexmap_clear(&m);
+    nalloc_x = 0;
+}
+
+gmx_ana_pos_t::~gmx_ana_pos_t()
+{
+    sfree(x);
+    sfree(v);
+    sfree(f);
+    gmx_ana_indexmap_deinit(&m);
 }
 
 /*!
@@ -173,7 +176,6 @@ gmx_ana_pos_reserve_for_append(gmx_ana_pos_t *pos, int n, int isize,
 void
 gmx_ana_pos_init_const(gmx_ana_pos_t *pos, const rvec x)
 {
-    gmx_ana_pos_clear(pos);
     snew(pos->x, 1);
     snew(pos->v, 1);
     snew(pos->f, 1);
@@ -184,39 +186,6 @@ gmx_ana_pos_init_const(gmx_ana_pos_t *pos, const rvec x)
     gmx_ana_indexmap_init(&pos->m, NULL, NULL, INDEX_UNKNOWN);
 }
 
-/*!
- * \param[in,out] pos   Position data structure.
- *
- * Frees any memory allocated within \p pos.
- * The pointer \p pos itself is not freed.
- *
- * \see gmx_ana_pos_free()
- */
-void
-gmx_ana_pos_deinit(gmx_ana_pos_t *pos)
-{
-    sfree(pos->x); pos->x = NULL;
-    sfree(pos->v); pos->v = NULL;
-    sfree(pos->f); pos->f = NULL;
-    pos->nalloc_x         = 0;
-    gmx_ana_indexmap_deinit(&pos->m);
-}
-
-/*!
- * \param[in,out] pos   Position data structure.
- *
- * Frees any memory allocated for \p pos.
- * The pointer \p pos is also freed, and is invalid after the call.
- *
- * \see gmx_ana_pos_deinit()
- */
-void
-gmx_ana_pos_free(gmx_ana_pos_t *pos)
-{
-    gmx_ana_pos_deinit(pos);
-    sfree(pos);
-}
-
 /*!
  * \param[in,out] dest   Destination positions.
  * \param[in]     src    Source positions.

diff --git a/src/gromacs/selection/position.h b/src/gromacs/selection/position.h
index f671dfc464df074fe85f18c96c9f631552d7a753..1e3b6e1a03fe4a28c8d07f69b28b895c5aecd29d 100644 (file)
--- a/src/gromacs/selection/position.h
+++ b/src/gromacs/selection/position.h
@@ -48,8 +48,12 @@
 /*! \brief
  * Stores a set of positions together with their origins.
  */
-typedef struct gmx_ana_pos_t
+struct gmx_ana_pos_t
 {
+    //! Initializes an empty position structure.
+    gmx_ana_pos_t();
+    ~gmx_ana_pos_t();
+
     //! Returns the number of positions.
     int count() const { return m.mapb.nr; }
 
@@ -75,11 +79,8 @@ typedef struct gmx_ana_pos_t
      * Number of elements allocated for \c x.
      */
     int                 nalloc_x;
-} gmx_ana_pos_t;
+};
 
-/** Initializes an empty position structure. */
-void
-gmx_ana_pos_clear(gmx_ana_pos_t *pos);
 /** Ensures that enough memory has been allocated to store positions. */
 void
 gmx_ana_pos_reserve(gmx_ana_pos_t *pos, int n, int isize);
@@ -96,12 +97,6 @@ gmx_ana_pos_reserve_for_append(gmx_ana_pos_t *pos, int n, int isize,
 /** Initializes a \c gmx_ana_pos_t to represent a constant position. */
 void
 gmx_ana_pos_init_const(gmx_ana_pos_t *pos, const rvec x);
-/** Frees the memory allocated for position storage. */
-void
-gmx_ana_pos_deinit(gmx_ana_pos_t *pos);
-/** Frees the memory allocated for positions. */
-void
-gmx_ana_pos_free(gmx_ana_pos_t *pos);
 /** Copies the evaluated positions to a preallocated data structure. */
 void
 gmx_ana_pos_copy(gmx_ana_pos_t *dest, gmx_ana_pos_t *src, bool bFirst);

diff --git a/src/gromacs/selection/selection.cpp b/src/gromacs/selection/selection.cpp
index 8155a2b4dd95ac3af8cbb02b848279c006dabb57..21967fbb3ada72f82a251e2565fc11f419e9603c 100644 (file)
--- a/src/gromacs/selection/selection.cpp
+++ b/src/gromacs/selection/selection.cpp
@@ -63,8 +63,6 @@ SelectionData::SelectionData(SelectionTreeElement *elem,
       coveredFraction_(1.0), averageCoveredFraction_(1.0),
       bDynamic_(false), bDynamicCoveredFraction_(false)
 {
-    gmx_ana_pos_clear(&rawPositions_);
-
     if (elem->child->type == SEL_CONST)
     {
         // TODO: This is not exception-safe if any called function throws.
@@ -107,7 +105,6 @@ SelectionData::SelectionData(SelectionTreeElement *elem,
 
 SelectionData::~SelectionData()
 {
-    gmx_ana_pos_deinit(&rawPositions_);
 }
 
 

diff --git a/src/gromacs/selection/selelem.cpp b/src/gromacs/selection/selelem.cpp
index e1996fd80b34c36e8a19f01f8ff477bee73d34da..eba3dffa6d79fbbeb4c42f224985d786093c6580 100644 (file)
--- a/src/gromacs/selection/selelem.cpp
+++ b/src/gromacs/selection/selelem.cpp
@@ -174,12 +174,6 @@ void SelectionTreeElement::freeValues()
                     sfree(v.u.s[i]);
                 }
                 break;
-            case POS_VALUE:
-                for (int i = 0; i < n; ++i)
-                {
-                    gmx_ana_pos_deinit(&v.u.p[i]);
-                }
-                break;
             case GROUP_VALUE:
                 for (int i = 0; i < n; ++i)
                 {
@@ -190,9 +184,16 @@ void SelectionTreeElement::freeValues()
                 break;
         }
     }
-    if (flags & SEL_ALLOCVAL)
+    if (v.nalloc > 0)
     {
-        sfree(v.u.ptr);
+        if (v.type == POS_VALUE)
+        {
+            delete [] v.u.p;
+        }
+        else
+        {
+            sfree(v.u.ptr);
+        }
     }
     _gmx_selvalue_setstore(&v, NULL);
     if (type == SEL_SUBEXPRREF && u.param)
@@ -210,11 +211,8 @@ SelectionTreeElement::freeExpressionData()
         u.expr.mdata  = NULL;
         u.expr.method = NULL;
         /* Free position data */
-        if (u.expr.pos)
-        {
-            gmx_ana_pos_free(u.expr.pos);
-            u.expr.pos = NULL;
-        }
+        delete u.expr.pos;
+        u.expr.pos = NULL;
         /* Free position calculation data */
         if (u.expr.pc)
         {
@@ -451,9 +449,10 @@ _gmx_selelem_free_method(gmx_ana_selmethod_t *method, void *mdata)
                 }
                 else if (param->val.type == POS_VALUE)
                 {
-                    for (j = 0; j < param->val.nr; ++j)
+                    if (param->val.nalloc > 0)
                     {
-                        gmx_ana_pos_deinit(&param->val.u.p[j]);
+                        delete[] param->val.u.p;
+                        _gmx_selvalue_setstore(&param->val, NULL);
                     }
                 }
 

diff --git a/src/gromacs/selection/selvalue.cpp b/src/gromacs/selection/selvalue.cpp
index 2f6f171efb3575c80a5c2bf18aabdd06ba9694c8..00b26a92caeaff5c4ed008aa79c898cb0b58ffac 100644 (file)
--- a/src/gromacs/selection/selvalue.cpp
+++ b/src/gromacs/selection/selvalue.cpp
@@ -44,6 +44,7 @@
 #include "gromacs/selection/indexutil.h"
 #include "gromacs/selection/position.h"
 #include "gromacs/selection/selvalue.h"
+#include "gromacs/utility/gmxassert.h"
 
 /*!
  * \param[out] val  Output structure
@@ -97,11 +98,9 @@ _gmx_selvalue_reserve(gmx_ana_selvalue_t *val, int n)
                 }
                 break;
             case POS_VALUE:
-                srenew(val->u.p, n);
-                for (i = val->nalloc; i < n; ++i)
-                {
-                    gmx_ana_pos_clear(&val->u.p[i]);
-                }
+                GMX_RELEASE_ASSERT(val->u.ptr == NULL,
+                                   "Reallocation of position values not supported");
+                val->u.p = new gmx_ana_pos_t[n];
                 break;
             case GROUP_VALUE:
                 srenew(val->u.g, n);

diff --git a/src/gromacs/selection/sm_distance.cpp b/src/gromacs/selection/sm_distance.cpp
index 8d3d0e7732c9254d6e972503114be0056a0a6c04..02fd047fd3059839496f7004a187dda0f7a5836e 100644 (file)
--- a/src/gromacs/selection/sm_distance.cpp
+++ b/src/gromacs/selection/sm_distance.cpp
@@ -60,7 +60,6 @@ struct t_methoddata_distance
 {
     t_methoddata_distance() : cutoff(-1.0)
     {
-        gmx_ana_pos_clear(&p);
     }
 
     /** Cutoff distance. */

diff --git a/src/gromacs/selection/sm_insolidangle.cpp b/src/gromacs/selection/sm_insolidangle.cpp
index 61300e01d68193dc6b1916586a679017a50a1e4c..563cecf849d6cc3e3472a07d7b2bd735001c63b0 100644 (file)
--- a/src/gromacs/selection/sm_insolidangle.cpp
+++ b/src/gromacs/selection/sm_insolidangle.cpp
@@ -331,10 +331,20 @@ gmx_ana_selmethod_t sm_insolidangle = {
 static void *
 init_data_insolidangle(int npar, gmx_ana_selparam_t *param)
 {
-    t_methoddata_insolidangle *data;
+    t_methoddata_insolidangle *data = new t_methoddata_insolidangle();
+    data->angcut        = 5.0;
+    data->cfrac         = 0.0;
+
+    data->distccut      = 0.0;
+    data->targetbinsize = 0.0;
+
+    data->ntbins        = 0;
+    data->tbinsize      = 0.0;
+    data->tbin          = NULL;
+    data->maxbins       = 0;
+    data->nbins         = 0;
+    data->bin           = NULL;
 
-    snew(data, 1);
-    data->angcut     = 5.0;
     param[0].val.u.p = &data->center;
     param[1].val.u.p = &data->span;
     param[2].val.u.r = &data->angcut;
@@ -406,7 +416,7 @@ free_data_insolidangle(void *data)
     }
     free_surface_points(d);
     sfree(d->bin);
-    sfree(d);
+    delete d;
 }
 
 /*!

diff --git a/src/gromacs/selection/sm_merge.cpp b/src/gromacs/selection/sm_merge.cpp
index 2ad9f02dc56bca2c2ec7090812235e49c28bca08..b015667ebf133b911be0f35d96acafb8b601b719 100644 (file)
--- a/src/gromacs/selection/sm_merge.cpp
+++ b/src/gromacs/selection/sm_merge.cpp
@@ -158,9 +158,7 @@ gmx_ana_selmethod_t sm_plus = {
 static void *
 init_data_merge(int npar, gmx_ana_selparam_t *param)
 {
-    t_methoddata_merge *data;
-
-    snew(data, 1);
+    t_methoddata_merge *data = new t_methoddata_merge();
     data->stride     = 0;
     param[0].val.u.p = &data->p1;
     param[1].val.u.p = &data->p2;
@@ -278,8 +276,7 @@ static void
 free_data_merge(void *data)
 {
     t_methoddata_merge *d = (t_methoddata_merge *)data;
-
-    sfree(d);
+    delete d;
 }
 
 /*!

diff --git a/src/gromacs/selection/sm_permute.cpp b/src/gromacs/selection/sm_permute.cpp
index cc84bf5c31a84fc2d6aff4be586923e28e97770b..07ac04ae85448b9279c35366bf9a6bba937f57c9 100644 (file)
--- a/src/gromacs/selection/sm_permute.cpp
+++ b/src/gromacs/selection/sm_permute.cpp
@@ -131,9 +131,10 @@ gmx_ana_selmethod_t sm_permute = {
 static void *
 init_data_permute(int npar, gmx_ana_selparam_t *param)
 {
-    t_methoddata_permute *data;
-
-    snew(data, 1);
+    t_methoddata_permute *data = new t_methoddata_permute();
+    data->n          = 0;
+    data->perm       = NULL;
+    data->rperm      = NULL;
     param[0].val.u.p = &data->p;
     return data;
 }
@@ -214,7 +215,7 @@ free_data_permute(void *data)
     t_methoddata_permute *d = (t_methoddata_permute *)data;
 
     sfree(d->rperm);
-    sfree(d);
+    delete d;
 }
 
 /*!

diff --git a/src/gromacs/selection/tests/poscalc.cpp b/src/gromacs/selection/tests/poscalc.cpp
index 40ffa76ef4514af4761dfd485c8396f7115bc032..aa7198b207155ff23b14a0383732291827763f47 100644 (file)
--- a/src/gromacs/selection/tests/poscalc.cpp
+++ b/src/gromacs/selection/tests/poscalc.cpp
@@ -50,6 +50,7 @@
 #include "gromacs/selection/indexutil.h"
 #include "gromacs/selection/poscalc.h"
 #include "gromacs/selection/position.h"
+#include "gromacs/utility/uniqueptr.h"
 
 #include "testutils/refdata.h"
 
@@ -121,16 +122,17 @@ class PositionCalculationTest : public ::testing::Test
         gmx::PositionCalculationCollection  pcc_;
 
     private:
+        typedef gmx::gmx_unique_ptr<gmx_ana_pos_t>::type PositionPointer;
+
         struct PositionTest
         {
-            PositionTest() : pos(NULL), pc(NULL), name(NULL) {}
-            PositionTest(gmx_ana_pos_t *pos, gmx_ana_poscalc_t *pc,
+            PositionTest(PositionPointer pos, gmx_ana_poscalc_t *pc,
                          const char *name)
-                : pos(pos), pc(pc), name(name)
+                : pos(gmx::move(pos)), pc(pc), name(name)
             {
             }
 
-            gmx_ana_pos_t                  *pos;
+            PositionPointer                 pos;
             gmx_ana_poscalc_t              *pc;
             const char                     *name;
         };
@@ -160,12 +162,6 @@ PositionCalculationTest::~PositionCalculationTest()
     {
         gmx_ana_poscalc_free(*pci);
     }
-
-    PositionTestList::iterator pi;
-    for (pi = posList_.begin(); pi != posList_.end(); ++pi)
-    {
-        gmx_ana_pos_free(pi->pos);
-    }
 }
 
 void PositionCalculationTest::generateCoordinates()
@@ -212,11 +208,11 @@ gmx_ana_pos_t *
 PositionCalculationTest::initPositions(gmx_ana_poscalc_t *pc, const char *name)
 {
     posList_.reserve(posList_.size() + 1);
-    gmx_ana_pos_t *p;
-    snew(p, 1);
-    posList_.push_back(PositionTest(p, pc, name));
-    gmx_ana_poscalc_init_pos(pc, p);
-    return p;
+    PositionPointer p(new gmx_ana_pos_t());
+    gmx_ana_pos_t  *result = p.get();
+    posList_.push_back(PositionTest(gmx::move(p), pc, name));
+    gmx_ana_poscalc_init_pos(pc, result);
+    return result;
 }
 
 void PositionCalculationTest::checkInitialized()
@@ -226,7 +222,7 @@ void PositionCalculationTest::checkInitialized()
     PositionTestList::const_iterator pi;
     for (pi = posList_.begin(); pi != posList_.end(); ++pi)
     {
-        checkPositions(&compound, pi->name, pi->pos, false);
+        checkPositions(&compound, pi->name, pi->pos.get(), false);
     }
 }