Reputation:
I have a function as below
int* readFile(string InputPath)
{
int *myvar = new int[10]; //The file has 10 lines (Using heap)
ifstream inFile;
inFile.open(InputPath.c_str(), ios::in);
if (inFile.fail())
{
cout << "Error reading the input file ";
cout << InputPath << ".";
exit(0);
}
string fileLine;
while (getline(inFile, fileLine))
{
myvar[i]=toint(fileLine); //will be converted to int!
}
;
inFile.close();
return myvar;
}:
How can I free the heap (myvar)? In general, what is the best method to return such array?
Upvotes: 1
Views: 2373
Reputation: 6200
The convention in C++ is to not return allocated memory. Instead, the function prototype should look like
size_t readFile(string InputPath,int* array,size_t n_elements);
The function returns the number of elements it actually placed in array. The caller will allocate and free memory using appropriate method, not nessecary new/delete[] but also malloc/free or lower level system functions such as VirtualAlloc.
Upvotes: 0
Reputation: 121971
The caller must delete[]
the value returned from the function. The code as it stands provides no protection for writing beyond the end of the array:
while (getline(inFile, fileLine))
{
myvar[i]=toint(fileLine); //will be converted to int!
}
However, as this is C++ use a std::vector<int>
instead and read int
s directly from the input stream instead of reading them as strings and performing the conversion. The std::vector<int>
will handle memory management for you:
std::vector<int> myvar;
int i;
while (inFile >> i) myvar.push_back(i);
Return the std::vector<int>
from the function. The caller can know exactly how many int
s are in the return value (which it cannot if you return an array unless you include a sentinel value to indicate the end) and does not need to explicitly delete it.
Upvotes: 2
Reputation: 3127
There must be some code which will call delete on this pointer.
I think, better way to do this is to get a pointer as argument. Doing it that way would force someone using this function to initialize array, so he would know, he has to delete it in the future.
Upvotes: 1
Reputation: 399863
It clearly becomes the responsibility of the caller, to call delete[]
on it. Note that this means the caller has to know that the returned pointer is allocated with new[]
, which isn't exactly optimal.
You should return a std::vector<int>
instead, which makes it all so much simpler.
Upvotes: 2
Reputation: 57322
How can I free the heap (myvar)?
The int* you return; don't change it, don't lose it, and when you're done with the memory,
delete [] theReturnedPointer;
Unless you've got a really good reason for making it an array, you could save yourself the bother of memory management and just use a vector.
best method
The best method is to return a vector:
vector<int> readFile(const string& InputPath)
{
ifstream inFile(InputPath); // or inputPath.c_str() for old compilers
if (!inFile)
{
cout << "Error reading the input file " << InputPath << ".";
exit(0); // thow would be better! Or at least return an empty vector.
}
vector<int> myvar;
for(int n; inFile >> n && myvar.size() < 10; )
{
myvar.push_back(n);
}
return myvar;
}
But if you really really want to use new[]
, then at least return the self-managing pointer, std::unique_ptr<int[]>
. Never let a raw pointer escape a function, not in C++.
Upvotes: 2