Skip to content

Fix memory leak in radTSend::OutRelaxResultsInfo #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
per-gron wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix memory leak in radTSend::OutRelaxResultsInfo #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions cpp/src/core/radsend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1957,15 +1957,15 @@ void radTSend::OutRelaxResultsInfo(double* RelaxStatusParamArray, int lenRelaxSt
//#ifdef __JAVA__
#if defined __JAVA__ || defined ALPHA__DLL__ || defined ALPHA__LIB__
int TotOutElem = lenRelaxStatusParamArray + 1;
double *TotOutArray = new double[TotOutElem];
if(TotOutArray == 0) { ErrorMessage("Radia::Error900"); return;}
double *t = TotOutArray;
std::vector<double> TotOutArray;
TotOutArray.resize(TotOutElem);
double *t = TotOutArray.data();
Copy link
Owner

@ochubar ochubar Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, and very good hint how to work with vector data via double* !
In this case, double* RelaxStatusParamArray can probably be directly submitted to MultiDimArrayOfDouble(..); I'll check this further and correct. Thanks!

Copy link
Author

@per-gron per-gron Dec 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to be of help.

If you want to give ownership of an object when calling a function, C++11 makes it possible to give objects like a vector to a function without expensive copying. For example:

void StoreMultiDimArrayOfDouble(std::vector<double> Array, int* Dims, int NumDims);

// And then call the function like this
StoreMultiDimArrayOfDouble(std::move(param), dims, n_dims);

Compared to passing a raw pointer with the soft contract that the ownership is given to the function, with this approach there is no ambiguity and much lower risk of mistakes.

If you want to go a little deeper, effectively the same thing could be done, ever so slightly more efficiently, with

void StoreMultiDimArrayOfDouble(std::vector<double>&& Array, int* Dims, int NumDims);

This is an "rvalue reference", a new kind of reference that is similar to a normal reference but it also means that the recipient of it is allowed to consume the object, the caller promises that the next method call is either the destructor or assignment operator. (std::move doesn't actually move, what it does is to return an rvalue reference to its parameter. Code that uses std::move should make sure to not use the moved object afterwards)

double *tRelaxStatusParamArray = RelaxStatusParamArray;
for(int i=0; i<lenRelaxStatusParamArray; i++) *(t++) = *(tRelaxStatusParamArray++);
*t = ActualIterNum;

int Dims[] = { TotOutElem};
MultiDimArrayOfDouble(TotOutArray, Dims, 1);
MultiDimArrayOfDouble(TotOutArray.data(), Dims, 1);
#endif
}

Expand Down