From 53c606f4a4c32848ca3f00985c33cf1f4546d0c5 Mon Sep 17 00:00:00 2001
From: Giovanni Bussi <giovanni.bussi@gmail.com>
Date: Fri, 1 Sep 2017 10:10:32 +0200
Subject: [PATCH] unique_ptr: generic and cltools

---
 src/cltools/Driver.cpp        | 20 +++++++++++---------
 src/cltools/SimpleMD.cpp      |  7 +++----
 src/cltools/pesmd.cpp         |  5 +++--
 src/generic/DumpAtoms.cpp     |  6 ++----
 src/generic/FitToTemplate.cpp | 13 ++++---------
 src/generic/Read.cpp          | 29 ++++++++++++++++-------------
 6 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/src/cltools/Driver.cpp b/src/cltools/Driver.cpp
index aa8a9deaf..e4a150740 100644
--- a/src/cltools/Driver.cpp
+++ b/src/cltools/Driver.cpp
@@ -30,6 +30,7 @@
 #include <cstring>
 #include <vector>
 #include <map>
+#include <memory>
 #include "tools/Units.h"
 #include "tools/PDB.h"
 #include "tools/FileBase.h"
@@ -287,10 +288,9 @@ int Driver<real>::main(FILE* in,FILE*out,Communicator& pc) {
   std::string fakein;
   bool debugfloat=parse("--debug-float",fakein);
   if(debugfloat && sizeof(real)!=sizeof(float)) {
-    CLTool* cl=cltoolRegister().create(CLToolOptions("driver-float"));    //new Driver<float>(*this);
+    std::unique_ptr<CLTool> cl(cltoolRegister().create(CLToolOptions("driver-float")));
     cl->setInputData(this->getInputData());
     int ret=cl->main(in,out,pc);
-    delete cl;
     return ret;
   }
 
@@ -358,7 +358,10 @@ int Driver<real>::main(FILE* in,FILE*out,Communicator& pc) {
   molfile_plugin_t *api=NULL;
   void *h_in=NULL;
   molfile_timestep_t ts_in; // this is the structure that has the timestep
-  ts_in.coords=NULL;
+// a std::unique_ptr<float> with the same scope as ts_in
+// it is necessary in order to store the pointer to ts_in.coords
+  std::unique_ptr<float[]> ts_in_coords;
+  ts_in.coords=ts_in_coords.get();
   ts_in.A=-1; // we use this to check whether cell is provided or not
 #endif
 
@@ -511,7 +514,8 @@ int Driver<real>::main(FILE* in,FILE*out,Communicator& pc) {
           if(command_line_natoms>=0) natoms=command_line_natoms;
           else error("this file format does not provide number of atoms; use --natoms on the command line");
         }
-        ts_in.coords = new float [3*natoms];
+        ts_in_coords.reset(new float [3*natoms]);
+        ts_in.coords = ts_in_coords.get();
 #endif
       } else if(trajectory_fmt=="xdr-xtc" || trajectory_fmt=="xdr-trr") {
 #ifdef __PLUMED_HAS_XDRFILE
@@ -739,18 +743,17 @@ int Driver<real>::main(FILE* in,FILE*out,Communicator& pc) {
         int localstep;
         float time;
         matrix box;
-        rvec* pos=new rvec[natoms];
+        std::unique_ptr<rvec[]> pos(new rvec[natoms]);
         float prec,lambda;
         int ret;
-        if(trajectory_fmt=="xdr-xtc") ret=read_xtc(xd,natoms,&localstep,&time,box,pos,&prec);
-        if(trajectory_fmt=="xdr-trr") ret=read_trr(xd,natoms,&localstep,&time,&lambda,box,pos,NULL,NULL);
+        if(trajectory_fmt=="xdr-xtc") ret=read_xtc(xd,natoms,&localstep,&time,box,pos.get(),&prec);
+        if(trajectory_fmt=="xdr-trr") ret=read_trr(xd,natoms,&localstep,&time,&lambda,box,pos.get(),NULL,NULL);
         if(stride==0) step=localstep;
         if(ret==exdrENDOFFILE) break;
         if(ret!=exdrOK) break;
         for(unsigned i=0; i<3; i++) for(unsigned j=0; j<3; j++) cell[3*i+j]=box[i][j];
         for(unsigned i=0; i<natoms; i++) for(unsigned j=0; j<3; j++)
             coordinates[3*i+j]=real(pos[i][j]);
-        delete [] pos;
 #endif
       } else {
         if(trajectory_fmt=="xyz") {
@@ -944,7 +947,6 @@ int Driver<real>::main(FILE* in,FILE*out,Communicator& pc) {
 #endif
 #ifdef __PLUMED_HAS_MOLFILE_PLUGINS
   if(h_in) api->close_file_read(h_in);
-  if(ts_in.coords) delete [] ts_in.coords;
 #endif
   if(grex_log) fclose(grex_log);
 
diff --git a/src/cltools/SimpleMD.cpp b/src/cltools/SimpleMD.cpp
index 78a938fa5..b8272f20c 100644
--- a/src/cltools/SimpleMD.cpp
+++ b/src/cltools/SimpleMD.cpp
@@ -28,6 +28,7 @@
 #include <cstdio>
 #include <cmath>
 #include <vector>
+#include <memory>
 
 using namespace std;
 
@@ -443,10 +444,10 @@ private:
 
     Random random;                 // random numbers stream
 
-    PLMD::Plumed* plumed=NULL;
+    std::unique_ptr<Plumed> plumed;
 
 // Commenting the next line it is possible to switch-off plumed
-    plumed=new PLMD::Plumed;
+    plumed.reset(new PLMD::Plumed);
 
     if(plumed) {
       int s=sizeof(double);
@@ -608,8 +609,6 @@ private:
 // close the statistic file if it was open:
     if(write_statistics_fp) fclose(write_statistics_fp);
 
-    if(plumed) delete plumed;
-
     return 0;
   }
 
diff --git a/src/cltools/pesmd.cpp b/src/cltools/pesmd.cpp
index 8479c73c7..210796b95 100755
--- a/src/cltools/pesmd.cpp
+++ b/src/cltools/pesmd.cpp
@@ -29,6 +29,7 @@
 #include <cstdio>
 #include <cmath>
 #include <vector>
+#include <memory>
 
 //+PLUMEDOC TOOLS pesmd
 /*
@@ -183,7 +184,8 @@ public:
     else if( lperiod ) error("invalid dimension for periodic potential must be 1, 2 or 3");
 
     // Create plumed object and initialize
-    PLMD::Plumed* plumed=new PLMD::Plumed; int s=sizeof(double);
+    std::unique_ptr<PLMD::Plumed> plumed(new PLMD::Plumed);
+    int s=sizeof(double);
     plumed->cmd("setRealPrecision",&s);
     if(Communicator::initialized()) plumed->cmd("setMPIComm",&pc.Get_comm());
     plumed->cmd("setNoVirial");
@@ -309,7 +311,6 @@ public:
       if( pc.Get_rank()==0 ) fprintf(fp,"%u %f %f %f \n", istep, istep*tstep, tke, therm_eng );
     }
 
-    delete plumed;
     fclose(fp);
 
     return 0;
diff --git a/src/generic/DumpAtoms.cpp b/src/generic/DumpAtoms.cpp
index bc98a58d2..f914d9bec 100644
--- a/src/generic/DumpAtoms.cpp
+++ b/src/generic/DumpAtoms.cpp
@@ -28,6 +28,7 @@
 #include "core/Atoms.h"
 #include "tools/Units.h"
 #include <cstdio>
+#include <memory>
 #include "core/SetupMolInfo.h"
 #include "core/ActionSet.h"
 
@@ -308,16 +309,13 @@ void DumpAtoms::update() {
     float time=getTime()/plumed.getAtoms().getUnits().getTime();
     float precision=Tools::fastpow(10.0,iprecision);
     for(int i=0; i<3; i++) for(int j=0; j<3; j++) box[i][j]=lenunit*t(i,j);
-    rvec* pos=new rvec [natoms];
-// Notice that code below cannot throw any exception.
-// Thus, this pointer is excepton safe
+    std::unique_ptr<rvec[]> pos(new rvec [natoms]);
     for(int i=0; i<natoms; i++) for(int j=0; j<3; j++) pos[i][j]=lenunit*getPosition(i)(j);
     if(type=="xtc") {
       write_xtc(xd,natoms,step,time,box,&pos[0],precision);
     } else if(type=="trr") {
       write_trr(xd,natoms,step,time,0.0,box,&pos[0],NULL,NULL);
     }
-    delete [] pos;
 #endif
   } else plumed_merror("unknown file type "+type);
 }
diff --git a/src/generic/FitToTemplate.cpp b/src/generic/FitToTemplate.cpp
index b227be8a1..2e45956b7 100644
--- a/src/generic/FitToTemplate.cpp
+++ b/src/generic/FitToTemplate.cpp
@@ -37,6 +37,7 @@
 
 #include <vector>
 #include <string>
+#include <memory>
 
 using namespace std;
 
@@ -155,7 +156,7 @@ class FitToTemplate:
   Vector center;
   Vector shift;
   // optimal alignment related stuff
-  PLMD::RMSD* rmsd;
+  std::unique_ptr<PLMD::RMSD> rmsd;
   Tensor rotation;
   Matrix< std::vector<Vector> > drotdpos;
   std::vector<Vector> positions;
@@ -167,7 +168,6 @@ class FitToTemplate:
 
 public:
   explicit FitToTemplate(const ActionOptions&ao);
-  ~FitToTemplate();
   static void registerKeywords( Keywords& keys );
   void calculate();
   void apply();
@@ -188,8 +188,7 @@ FitToTemplate::FitToTemplate(const ActionOptions&ao):
   Action(ao),
   ActionPilot(ao),
   ActionAtomistic(ao),
-  ActionWithValue(ao),
-  rmsd(NULL)
+  ActionWithValue(ao)
 {
   string reference;
   parse("REFERENCE",reference);
@@ -231,7 +230,7 @@ FitToTemplate::FitToTemplate(const ActionOptions&ao):
   for(unsigned i=0; i<weights.size(); ++i) positions[i]-=center;
 
   if(type=="OPTIMAL" or type=="OPTIMAL-FAST" ) {
-    rmsd=new RMSD();
+    rmsd.reset(new RMSD());
     rmsd->set(weights,weights_measure,positions,type,false,false);// note: the reference is shifted now with center in the origin
     log<<"  Method chosen for fitting: "<<rmsd->getMethod()<<" \n";
   }
@@ -331,9 +330,5 @@ void FitToTemplate::apply() {
   }
 }
 
-FitToTemplate::~FitToTemplate() {
-  if(rmsd) delete rmsd;
-}
-
 }
 }
diff --git a/src/generic/Read.cpp b/src/generic/Read.cpp
index bb81b915e..e6dee365e 100644
--- a/src/generic/Read.cpp
+++ b/src/generic/Read.cpp
@@ -26,6 +26,7 @@
 #include "core/ActionSet.h"
 #include "core/Atoms.h"
 #include "tools/IFile.h"
+#include <memory>
 
 namespace PLMD {
 namespace generic {
@@ -71,12 +72,16 @@ private:
   bool cloned_file;
   unsigned nlinesPerStep;
   std::string filename;
+/// Unique pointer with the same scope as ifile.
+  std::unique_ptr<IFile> ifile_ptr;
+/// Pointer to input file.
+/// It is either pointing to the content of ifile_ptr
+/// or to the file it is cloned from.
   IFile* ifile;
-  std::vector<Value*> readvals;
+  std::vector<std::unique_ptr<Value>> readvals;
 public:
   static void registerKeywords( Keywords& keys );
   explicit Read(const ActionOptions&);
-  ~Read();
   void prepare();
   void apply() {}
   void calculate();
@@ -131,7 +136,8 @@ Read::Read(const ActionOptions&ao):
     }
   }
   if( !cloned_file ) {
-    ifile=new IFile();
+    ifile_ptr.reset(new IFile());
+    ifile=ifile_ptr.get();
     if( !ifile->FileExist(filename) ) error("could not find file named " + filename);
     ifile->link(*this);
     ifile->open(filename);
@@ -153,25 +159,25 @@ Read::Read(const ActionOptions&ao):
       ifile->scanFieldList( fieldnames );
       for(unsigned i=0; i<fieldnames.size(); ++i) {
         if( fieldnames[i].substr(0,dot)==label ) {
-          readvals.push_back(new Value(this, fieldnames[i], false) ); addComponentWithDerivatives( fieldnames[i].substr(dot+1) );
+          readvals.emplace_back(new Value(this, fieldnames[i], false) ); addComponentWithDerivatives( fieldnames[i].substr(dot+1) );
           if( ifile->FieldExist("min_" + fieldnames[i]) ) componentIsPeriodic( fieldnames[i].substr(dot+1), "-pi","pi" );
           else componentIsNotPeriodic( fieldnames[i].substr(dot+1) );
         }
       }
     } else {
-      readvals.push_back(new Value(this, valread[0], false) ); addComponentWithDerivatives( name );
+      readvals.emplace_back(new Value(this, valread[0], false) ); addComponentWithDerivatives( name );
       if( ifile->FieldExist("min_" + valread[0]) ) componentIsPeriodic( valread[0].substr(dot+1), "-pi", "pi" );
       else componentIsNotPeriodic( valread[0].substr(dot+1) );
       for(unsigned i=1; i<valread.size(); ++i) {
         if( valread[i].substr(0,dot)!=label ) error("all values must be from the same Action when using READ");;
-        readvals.push_back(new Value(this, valread[i], false) ); addComponentWithDerivatives( valread[i].substr(dot+1) );
+        readvals.emplace_back(new Value(this, valread[i], false) ); addComponentWithDerivatives( valread[i].substr(dot+1) );
         if( ifile->FieldExist("min_" + valread[i]) ) componentIsPeriodic( valread[i].substr(dot+1), "-pi", "pi" );
         else componentIsNotPeriodic( valread[i].substr(dot+1) );
       }
     }
   } else {
     if( valread.size()!=1 ) error("all values must be from the same Action when using READ");
-    readvals.push_back(new Value(this, valread[0], false) ); addValueWithDerivatives();
+    readvals.emplace_back(new Value(this, valread[0], false) ); addValueWithDerivatives();
     if( ifile->FieldExist("min_" + valread[0]) ) setPeriodic( "-pi", "pi" );
     else setNotPeriodic();
     log.printf("  reading value %s and storing as %s\n",valread[0].c_str(),getLabel().c_str() );
@@ -179,11 +185,6 @@ Read::Read(const ActionOptions&ao):
   checkRead();
 }
 
-Read::~Read() {
-  if( !cloned_file ) { ifile->close(); delete ifile; }
-  for(unsigned i=0; i<readvals.size(); ++i) delete readvals[i];
-}
-
 std::string Read::getFilename() const {
   return filename;
 }
@@ -216,7 +217,9 @@ void Read::prepare() {
 void Read::calculate() {
   std::string smin, smax;
   for(unsigned i=0; i<readvals.size(); ++i) {
-    ifile->scanField( readvals[i] );
+// .get  returns the raw pointer
+// ->get calls the Value::get() method
+    ifile->scanField( readvals[i].get() );
     getPntrToComponent(i)->set( readvals[i]->get() );
     if( readvals[i]->isPeriodic() ) {
       readvals[i]->getDomain( smin, smax );
-- 
GitLab