Re: [cctbxbb] [git/cctbx] master: Replacing illegal C++ code with proper dynamic array allocation. (a31eb2bf0)
Rob, Just as a general thing, "new" statements in C++ need to be paired with "delete" statements (delete pk, delete a_sum, and delete akml). Unless this is done there will be a memory leak. Nick Nicholas K. Sauter, Ph. D. Senior Scientist, Molecular Biophysics & Integrated Bioimaging Division Lawrence Berkeley National Laboratory 1 Cyclotron Rd., Bldg. 33R0345 Berkeley, CA 94720 (510) 486-5713 On Tue, Apr 11, 2017 at 3:31 AM, CCTBX Commit via DLS Jenkins < [email protected]> wrote:
Repository : ssh://g18-sc-serv-04.diamond.ac.uk/cctbx On branch : master
------------------------------
commit a31eb2bf0d5bb129425bb759466f45622c773fd4 Author: Robert Oeffner
Date: Tue Apr 11 11:31:24 2017 +0100 Replacing illegal C++ code with proper dynamic array allocation.
------------------------------
a31eb2bf0d5bb129425bb759466f45622c773fd4 scitbx/lbfgs.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scitbx/lbfgs.h b/scitbx/lbfgs.h index 41be8e8cc..f0000143d 100755 --- a/scitbx/lbfgs.h +++ b/scitbx/lbfgs.h @@ -917,9 +917,9 @@ namespace lbfgs { int info = 5; brackt = false; FloatType sxnorm = std::sqrt(ddot(SizeType(n), sx, sx)); - FloatType pk[n]; - FloatType a_sum[n]; - FloatType akm1[n]; + FloatType* pk = new FloatType[n]; + FloatType* a_sum = new FloatType[n]; + FloatType* akm1 = new FloatType[n]; for (SizeType i=0; i < n; i++) { pk[i] = sx[i]/sxnorm; akm1[i] = -gx[i]/sxnorm;
I reckon the delete statements should take place end of that function where I put in the new statements. The reason why I didn't put in delete statements is that I didn't author the original function and don't know its functionality. I was merely interested in fixing a broken Windows build. Currently the Windows build is broken again due to yesterdays introduction of libtiff in the base components. Once I've sorted that out I'm happy tentatively to put in the aforementioned delete statements unless someone else beats me. Rob On 11/04/2017 20:39, Nicholas Sauter wrote:
Rob,
Just as a general thing, "new" statements in C++ need to be paired with "delete" statements (delete pk, delete a_sum, and delete akml). Unless this is done there will be a memory leak.
Nick
Nicholas K. Sauter, Ph. D. Senior Scientist, Molecular Biophysics & Integrated Bioimaging Division Lawrence Berkeley National Laboratory 1 Cyclotron Rd., Bldg. 33R0345 Berkeley, CA 94720 (510) 486-5713
On Tue, Apr 11, 2017 at 3:31 AM, CCTBX Commit via DLS Jenkins
wrote: Repository : ssh://g18-sc-serv-04.diamond.ac.uk/cctbx [1] On branch : master
-------------------------
commit a31eb2bf0d5bb129425bb759466f45622c773fd4 Author: Robert Oeffner
Date: Tue Apr 11 11:31:24 2017 +0100 Replacing illegal C++ code with proper dynamic array allocation.
-------------------------
a31eb2bf0d5bb129425bb759466f45622c773fd4 scitbx/lbfgs.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scitbx/lbfgs.h b/scitbx/lbfgs.h index 41be8e8cc..f0000143d 100755 --- a/scitbx/lbfgs.h +++ b/scitbx/lbfgs.h @@ -917,9 +917,9 @@ namespace lbfgs { int info = 5; brackt = false; FloatType sxnorm = std::sqrt(ddot(SizeType(n), sx, sx)); - FloatType pk[n]; - FloatType a_sum[n]; - FloatType akm1[n]; + FloatType* pk = new FloatType[n]; + FloatType* a_sum = new FloatType[n]; + FloatType* akm1 = new FloatType[n]; for (SizeType i=0; i < n; i++) { pk[i] = sx[i]/sxnorm; akm1[i] = -gx[i]/sxnorm;
Links: ------ [1] http://g18-sc-serv-04.diamond.ac.uk/cctbx
Perhaps a std::vector would be a better alternative? It can handle dynamic
allocation and there is no need for a delete[] statement. This may depend
on the code using it, but it is worth a try.
- FloatType pk[n];
+ std::vector< FloatType > pk(n);
BW, Gabor
On Wed, Apr 12, 2017 at 12:20 PM, R.D. Oeffner
I reckon the delete statements should take place end of that function where I put in the new statements. The reason why I didn't put in delete statements is that I didn't author the original function and don't know its functionality. I was merely interested in fixing a broken Windows build.
Currently the Windows build is broken again due to yesterdays introduction of libtiff in the base components. Once I've sorted that out I'm happy tentatively to put in the aforementioned delete statements unless someone else beats me.
Rob
On 11/04/2017 20:39, Nicholas Sauter wrote:
Rob,
Just as a general thing, "new" statements in C++ need to be paired with "delete" statements (delete pk, delete a_sum, and delete akml). Unless this is done there will be a memory leak.
Nick
Nicholas K. Sauter, Ph. D. Senior Scientist, Molecular Biophysics & Integrated Bioimaging Division Lawrence Berkeley National Laboratory 1 Cyclotron Rd., Bldg. 33R0345 Berkeley, CA 94720 (510) 486-5713
On Tue, Apr 11, 2017 at 3:31 AM, CCTBX Commit via DLS Jenkins < [email protected]> wrote:
Repository : ssh://g18-sc-serv-04.diamond.ac.uk/cctbx On branch : master
------------------------------
commit a31eb2bf0d5bb129425bb759466f45622c773fd4 Author: Robert Oeffner
Date: Tue Apr 11 11:31:24 2017 +0100 Replacing illegal C++ code with proper dynamic array allocation.
------------------------------
a31eb2bf0d5bb129425bb759466f45622c773fd4 scitbx/lbfgs.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scitbx/lbfgs.h b/scitbx/lbfgs.h index 41be8e8cc..f0000143d 100755 --- a/scitbx/lbfgs.h +++ b/scitbx/lbfgs.h @@ -917,9 +917,9 @@ namespace lbfgs { int info = 5; brackt = false; FloatType sxnorm = std::sqrt(ddot(SizeType(n), sx, sx)); - FloatType pk[n]; - FloatType a_sum[n]; - FloatType akm1[n]; + FloatType* pk = new FloatType[n]; + FloatType* a_sum = new FloatType[n]; + FloatType* akm1 = new FloatType[n]; for (SizeType i=0; i < n; i++) { pk[i] = sx[i]/sxnorm; akm1[i] = -gx[i]/sxnorm;
_______________________________________________ cctbxbb mailing list [email protected] http://phenix-online.org/mailman/listinfo/cctbxbb
participants (3)
-
Gabor Bunkoczi
-
Nicholas Sauter
-
R.D. Oeffner