How to delete an array of pointers

Question

Hello all, I've been brushing up on my C++ as of late, and I have a quick question regarding the deletion of new'd memory. As you can see below i have a simple class that holds a list of FileData *. I created an array to hold the FileData objects to be pushed into the list. When ReportData is destructed I loop through the list and delete each element. My question is, how can i delete the array when I'm done using reportData, so that I do not have any memory leaks?

Report.h

class REPORTAPI ReportData {
public:

        ReportData()
        {
        }

        virtual ~ReportData()
        {
                printf("Starting ReportData Delete\n");
                for (list<FileData*>::iterator i = ReportFileData.begin(), e = ReportFileData.end(); i != e; )
                {
                    list<FileData*>::iterator tmp(i++);
                    delete *tmp;
                    ReportFileData.erase(tmp);
                }

                for (list<SupressionData*>::iterator i = ReportSupressionData.begin(), e = ReportSupressionData.end(); i != e; )
                {
                    list<SupressionData*>::iterator tmp(i++);
                    delete *tmp;
                    ReportSupressionData.erase(tmp);
                }

                ReportFileData.clear();
                ReportSupressionData.clear();

                printf("Finished ReportData Delete\n");
        }

        list<FileData *> ReportFileData;
        list<SupressionData *> ReportSupressionData;
}


extern "C" __declspec(dllexport) FileData* __stdcall createFileData(string fileName, long recordCount, long addPageCount)
{
        return new FileData(fileName, recordCount, addPageCount);
}

Main.cpp

        ReportData *reportData = createrd();

        if (reportData != NULL)
        {
                CreateFileDataFunc createfd (reinterpret_cast<CreateFileDataFunc>(GetProcAddress (dll, "createFileData")));

                const int num_files = 5;
                FileData *fileData[num_files];

                char buff[256] = {'\0'};
                for (int i = 0; i < num_files; i++)
                {
                        sprintf(buff, "test: %d", i);
                        fileData[i] = createfd(buff, 1, 1);
                        reportData->ReportFileData.push_back(fileData[i]);
                }

                delete reportData;
                reportData = NULL;

                delete [] fileData; // this is throwing an access violation error:
                                    //EAccessViolation: 'Access violation at address 326025AF. Write of address 00000008'.


        }

--- I removed the delete oprations from the ReportData dtor and I'm now looping and deleting:

            for(int i = 0; i < num_files; i++)
            {
                delete fileData[i];
            }

This is easier to understand then having to rely on a separate object's dtor to clean up memory.

Answer 1

You don't. fileData is an automatic (stack) variable. You didn't allocate it with new, so you don't delete it.

[Edit: also I'm not sure, but I think you could face problems deleting those FileData objects from main.cpp, considering that they were allocated in some dll. Does the dll provide a deleter function?]

Answer 2

// allocate on the stack, no manual delete required
FileData *fileData[num_files];

// allocate on the heap, must delete after usage
FileData *fileData = new FileData[num_files];
// ..
delete [] fileData;

Answer 3

Your array is not dynamically allocated, so you don't need to delete it. Each element, however, is pointing to a dynamically allocated object (from your comment):

createfd is a function pointer that returns a new instance of FileData though

What you need to do is loop over the elements of the array, and free each of them.

for(int i = 0; i < num_files; i++)
{
    delete fileData[i];
}

Answer 4

There is no need to dlete or clear either of the two lists - this will be done for you by the default destructor. assuming the pointers the lists contain (to "arrays"? I'm not clear) have been dynamicall allocated, you need to delete them. However, you can (and should) avoid having to do this expliciltly by making the lists contain std::vectors or suitable smart pointers.

Answer 5

"My question is, how can i delete the array when I'm done using reportData, so that I do not have any memory leaks?"

That's the wrong question. The right question is "who should delete these FileData objects?", and the answer is "whoever constructs them, ideally, in this cae Main.cpp". Farming out the job to reportData is awkward and precarious; doing the job twice (once in the ReportData destructor and again in Main.cpp) violates memory.

If you must destroy the objects in ~ReportData(), just don't do anything about them in Main.cpp. Then your code will be correct. Horrible, but correct.

Answer 6

Have you thought about wrapping FileData* with a smart pointer?

The problem with your dtor is that an exception will cause a memory leak (with some other problems relating to exceptions leaking out of dtor's).

Answer 7

Don't deallocate anything in main().

The destructor for reportData will handle everything allocated with createfd() (just make sure that createfd() is returning what it allocated with new(), since you must not delete anything that was not new'd).

fileData is allocated locally, on the stack, not through new. Since it wasn't allocated by new, don't delete it.

The pointers that were passed into fileData were also passed into reportData, and reportData is responsible for all deletions there. You could check to see that they weren't allocated from an inaccessible memory pool (say in a dynamically linked library); if they were, that's a problem.

So, assuming the deletes are correct in the ReportData destructor, remove any deletion of fileData and you're good.