From 05c44ecaaaecc46451a96022a9aac2850a3f5e75 Mon Sep 17 00:00:00 2001
From: Cristian C Lalescu <Cristian.Lalescu@ds.mpg.de>
Date: Sun, 17 Feb 2019 13:28:10 +0100
Subject: [PATCH] partial fix

when using unique_ptr for arrays, they are cleaned up by calling
`delete[] bla.release()`.
At least, cppreference says that the release method
"returns a pointer to the managed object and releases the ownership ".
https://en.cppreference.com/w/cpp/memory/unique_ptr
---
 bfps/cpp/full_code/NSVEcomplex_particles.cpp     |  7 +++----
 bfps/cpp/full_code/NSVEparticles.cpp             |  7 +++----
 bfps/cpp/full_code/test_interpolation.cpp        |  6 ++++--
 bfps/cpp/hdf5_tools.cpp                          |  3 +++
 bfps/cpp/particles/abstract_particles_output.hpp | 12 ++++++------
 bfps/cpp/particles/p2p_distr_mpi.hpp             |  6 +++---
 bfps/cpp/particles/particles_distr_mpi.hpp       | 10 +++++-----
 bfps/cpp/particles/particles_input_hdf5.hpp      |  4 ++--
 8 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/bfps/cpp/full_code/NSVEcomplex_particles.cpp b/bfps/cpp/full_code/NSVEcomplex_particles.cpp
index c7bc9424..3bd27102 100644
--- a/bfps/cpp/full_code/NSVEcomplex_particles.cpp
+++ b/bfps/cpp/full_code/NSVEcomplex_particles.cpp
@@ -229,10 +229,9 @@ int NSVEcomplex_particles<rnumber>::do_stats()
             this->ps->get_step_idx()-1);
 
     // deallocate temporary data array
-    // TODO: is it required/safe to call the release method here?
-    pdata0.release();
-    pdata1.release();
-    pdata2.release();
+    delete[] pdata0.release();
+    delete[] pdata1.release();
+    delete[] pdata2.release();
 
     return EXIT_SUCCESS;
 }
diff --git a/bfps/cpp/full_code/NSVEparticles.cpp b/bfps/cpp/full_code/NSVEparticles.cpp
index c8918996..9b8743cd 100644
--- a/bfps/cpp/full_code/NSVEparticles.cpp
+++ b/bfps/cpp/full_code/NSVEparticles.cpp
@@ -105,7 +105,7 @@ int NSVEparticles<rnumber>::finalize(void)
 {
     TIMEZONE("NSVEparticles::finalize");
     delete this->pressure;
-    this->ps.release();
+    delete this->ps.release();
     delete this->particles_output_writer_mpi;
     delete this->particles_sample_writer_mpi;
     this->NSVE<rnumber>::finalize();
@@ -134,7 +134,7 @@ int NSVEparticles<rnumber>::do_stats()
     // allocate temporary data array
     std::unique_ptr<double[]> pdata(new double[3*this->ps->getLocalNbParticles()]);
 
-    // copy position data
+    /// copy position data
 
     /// sample position
     std::copy(this->ps->getParticlesState(),
@@ -183,8 +183,7 @@ int NSVEparticles<rnumber>::do_stats()
             this->ps->get_step_idx()-1);
 
     // deallocate temporary data array
-    // TODO: is it required/safe to call the release method here?
-    pdata.release();
+    delete[] pdata.release();
 
     return EXIT_SUCCESS;
 }
diff --git a/bfps/cpp/full_code/test_interpolation.cpp b/bfps/cpp/full_code/test_interpolation.cpp
index 2acd3c27..c3103fb4 100644
--- a/bfps/cpp/full_code/test_interpolation.cpp
+++ b/bfps/cpp/full_code/test_interpolation.cpp
@@ -92,7 +92,7 @@ int test_interpolation<rnumber>::finalize(void)
     delete this->nabla_u;
     delete this->velocity;
     delete this->vorticity;
-    this->ps.release();
+    delete this->ps.release();
     delete this->kk;
     delete particles_output_writer_mpi;
     delete particles_sample_writer_mpi;
@@ -198,7 +198,9 @@ int test_interpolation<rnumber>::do_work()
             this->ps->getLocalNbParticles(),
             this->ps->get_step_idx()-1);
 
-    // no need to deallocate because we used "unique_ptr"
+    // deallocate temporary arrays
+    delete[] p3data.release();
+    delete[] p9data.release();
     return EXIT_SUCCESS;
 }
 
diff --git a/bfps/cpp/hdf5_tools.cpp b/bfps/cpp/hdf5_tools.cpp
index 25acaf21..5a3aef39 100644
--- a/bfps/cpp/hdf5_tools.cpp
+++ b/bfps/cpp/hdf5_tools.cpp
@@ -213,6 +213,9 @@ std::string hdf5_tools::read_string(
         hid_t dset = H5Dopen(group, dset_name.c_str(), H5P_DEFAULT);
         hid_t space = H5Dget_space(dset);
         hid_t memtype = H5Dget_type(dset);
+        // fsanitize complains unless I have a static array here
+        // but that doesn't actually work (data is read incorrectly).
+        // this is caught by bfps.test_NSVEparticles
         char *string_data = (char*)malloc(256);
         H5Dread(dset, memtype, H5S_ALL, H5S_ALL, H5P_DEFAULT, &string_data);
         std::string std_string_data = std::string(string_data);
diff --git a/bfps/cpp/particles/abstract_particles_output.hpp b/bfps/cpp/particles/abstract_particles_output.hpp
index 4fc344d3..a457689a 100644
--- a/bfps/cpp/particles/abstract_particles_output.hpp
+++ b/bfps/cpp/particles/abstract_particles_output.hpp
@@ -137,15 +137,15 @@ public:
     }
 
     void releaseMemory(){
-        buffer_indexes_send.release();
-        buffer_particles_positions_send.release();
+        delete[] buffer_indexes_send.release();
+        delete[] buffer_particles_positions_send.release();
         size_buffers_send = 0;
-        buffer_indexes_recv.release();
-        buffer_particles_positions_recv.release();
+        delete[] buffer_indexes_recv.release();
+        delete[] buffer_particles_positions_recv.release();
         size_buffers_recv = 0;
         for(int idx_rhs = 0 ; idx_rhs < nb_rhs ; ++idx_rhs){
-            buffer_particles_rhs_send[idx_rhs].release();
-            buffer_particles_rhs_recv[idx_rhs].release();
+            delete[] buffer_particles_rhs_send[idx_rhs].release();
+            delete[] buffer_particles_rhs_recv[idx_rhs].release();
         }
         buffers_size_particle_rhs_send = 0;
         buffers_size_particle_rhs_recv = 0;
diff --git a/bfps/cpp/particles/p2p_distr_mpi.hpp b/bfps/cpp/particles/p2p_distr_mpi.hpp
index 965f8ba8..9b8a5b13 100644
--- a/bfps/cpp/particles/p2p_distr_mpi.hpp
+++ b/bfps/cpp/particles/p2p_distr_mpi.hpp
@@ -625,7 +625,7 @@ public:
                         AssertMpi(MPI_Isend(descriptor.results.get(), int(NbParticlesToReceive*size_particle_rhs),
                                             particles_utils::GetMpiType(real_number()), destProc, TAG_RESULT_PARTICLES,
                                             current_com, &mpiRequests.back()));
-                        descriptor.toCompute.release();
+                        delete[] descriptor.toCompute.release();
                     }
                     //////////////////////////////////////////////////////////////////////
                     /// Release memory that was sent back
@@ -634,7 +634,7 @@ public:
                         NeighborDescriptor& descriptor = neigDescriptors[releasedAction.second];
                         assert(descriptor.results != nullptr);
                         assert(descriptor.isRecv);
-                        descriptor.results.release();
+                        delete[] descriptor.results.release();
                     }
                     //////////////////////////////////////////////////////////////////////
                     /// Merge
@@ -646,7 +646,7 @@ public:
                         assert(descriptor.toRecvAndMerge != nullptr);
                         in_computer.template reduce_particles_rhs<size_particle_rhs>(&particles_current_rhs[particles_offset_layers[my_nb_cell_levels-descriptor.nbLevelsToExchange]*size_particle_rhs],
                                 descriptor.toRecvAndMerge.get(), descriptor.nbParticlesToExchange);
-                        descriptor.toRecvAndMerge.release();
+                        delete[] descriptor.toRecvAndMerge.release();
                     }
                 }
             }
diff --git a/bfps/cpp/particles/particles_distr_mpi.hpp b/bfps/cpp/particles/particles_distr_mpi.hpp
index cec734e1..8a2b77ca 100644
--- a/bfps/cpp/particles/particles_distr_mpi.hpp
+++ b/bfps/cpp/particles/particles_distr_mpi.hpp
@@ -418,7 +418,7 @@ public:
                     if(releasedAction.first == RELEASE_BUFFER_PARTICLES){
                         NeighborDescriptor& descriptor = neigDescriptors[releasedAction.second];
                         assert(descriptor.toCompute != nullptr);
-                        descriptor.toCompute.release();
+                        delete[] descriptor.toCompute.release();
                     }
                     //////////////////////////////////////////////////////////////////////
                     /// Merge
@@ -430,14 +430,14 @@ public:
                             TIMEZONE("reduce");
                             assert(descriptor.toRecvAndMerge != nullptr);
                             in_computer.template reduce_particles_rhs<size_particle_rhs>(&particles_current_rhs[0], descriptor.toRecvAndMerge.get(), descriptor.nbParticlesToSend);
-                            descriptor.toRecvAndMerge.release();
+                            delete[] descriptor.toRecvAndMerge.release();
                         }
                         else {
                             TIMEZONE("reduce");
                             assert(descriptor.toRecvAndMerge != nullptr);
                             in_computer.template reduce_particles_rhs<size_particle_rhs>(&particles_current_rhs[(current_offset_particles_for_partition[current_partition_size]-descriptor.nbParticlesToSend)*size_particle_rhs],
                                              descriptor.toRecvAndMerge.get(), descriptor.nbParticlesToSend);
-                            descriptor.toRecvAndMerge.release();
+                            delete[] descriptor.toRecvAndMerge.release();
                         }
                     }
                 }
@@ -475,14 +475,14 @@ public:
                         TIMEZONE("reduce_later");
                         assert(descriptor.toRecvAndMerge != nullptr);
                         in_computer.template reduce_particles_rhs<size_particle_rhs>(&particles_current_rhs[0], descriptor.toRecvAndMerge.get(), descriptor.nbParticlesToSend);
-                        descriptor.toRecvAndMerge.release();
+                        delete[] descriptor.toRecvAndMerge.release();
                     }
                     else {
                         TIMEZONE("reduce_later");
                         assert(descriptor.toRecvAndMerge != nullptr);
                         in_computer.template reduce_particles_rhs<size_particle_rhs>(&particles_current_rhs[(current_offset_particles_for_partition[current_partition_size]-descriptor.nbParticlesToSend)*size_particle_rhs],
                                          descriptor.toRecvAndMerge.get(), descriptor.nbParticlesToSend);
-                        descriptor.toRecvAndMerge.release();
+                        delete[] descriptor.toRecvAndMerge.release();
                     }
                 }
             }
diff --git a/bfps/cpp/particles/particles_input_hdf5.hpp b/bfps/cpp/particles/particles_input_hdf5.hpp
index e10377bf..33406314 100644
--- a/bfps/cpp/particles/particles_input_hdf5.hpp
+++ b/bfps/cpp/particles/particles_input_hdf5.hpp
@@ -302,13 +302,13 @@ public:
                 my_particles_positions.reset(new real_number[exchanger.getTotalToRecv()*size_particle_positions]);
             }
             exchanger.alltoallv<real_number>(split_particles_positions.get(), my_particles_positions.get(), size_particle_positions);
-            split_particles_positions.release();
+            delete[] split_particles_positions.release();
 
             if(nb_particles_for_me){
                 my_particles_indexes.reset(new partsize_t[exchanger.getTotalToRecv()]);
             }
             exchanger.alltoallv<partsize_t>(split_particles_indexes.get(), my_particles_indexes.get());
-            split_particles_indexes.release();
+            delete[] split_particles_indexes.release();
 
             my_particles_rhs.resize(nb_rhs);
             for(int idx_rhs = 0 ; idx_rhs < int(nb_rhs) ; ++idx_rhs){
-- 
GitLab