user1679956
user1679956

Reputation:

delete heap after returning pointer

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

Answers (5)

user877329
user877329

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

hmjd
hmjd

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 ints 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 ints 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

Ari
Ari

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

unwind
unwind

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

NullPoiиteя
NullPoiиteя

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

Related Questions