Home > Enterprise >  C "pointer being freed was not allocated" Error
C "pointer being freed was not allocated" Error

Time:02-02

I have the following code:

main.cpp

MovieLibrary* library = new MovieLibrary(maxMovies);
delete[] library; // This throws the error.

MovieLibrary.cpp

MovieLibrary::MovieLibrary(int maxMovies) {
    this->maxMovies = maxMovies;
    this->numMovies = 0;
    this->movies = new Movie*[maxMovies];
}

MovieLibrary::~MovieLibrary() {
    for (int i=0; i<this->numMovies; i  ) {
        delete this->movies[i];
    }
    delete[] this->movies;
}

Movie.cpp

Movie::Movie(Text* title, Text* publisher, int year) {
    this->title = title;
    this->publisher = publisher;
    this->year = year;
}

This is not all the code, but it's all the code I believe should be required to find the cause of the error. I'm pretty novice to C and dynamic memory allocation. Running the program gives me a "pointer being freed was not allocated" and I've researched but to no avail. Most posts just say it's trying to clear something not initialized with malloc or new, but I mean MovieLibrary* library = new MovieLibrary(maxMovies); is initialized with new, so I'm lost.

Thank you in advance.

CodePudding user response:

As per @PeteBecker and @RemyLebeau:

Use delete library;, rather than delete[] library;

CodePudding user response:

First, it seems that you meant to use square brackets with new:

MovieLibrary* library = new MovieLibrary[maxMovies];
delete[] library; // This should not throw an error.

You should almost never use delete and new directly. There are complicated set of cases when you would still want to use new and delete directly, but this is not one of them.

Instead, prefer to use standard containers such as std::vector:

std::vector<MovieLibrary> library(maxMovies);
// no delete[] library, since std::vector does it for you

If you are not allowed to use std::vector then the next best thing in this case are std::unique_ptr and std::make_unique(). With these, the chance of making the same mistake as in your question are much smaller. This solution for dynamic arrays has been available since C 14 (for regular pointers since C 11):

std::unique_ptr<MovieLibrary[]> library = std::make_unique<MovieLibrary[]>(maxMovies);
// no need to delete library, it will be done for you at scope exit.

Besides, the code has other messed up things. It seems that you wanted to create a dynamic array of pointers to objects, but you attempted to create a dynamic array of objects instead. Please clarify what you intend to do. It is likely that you really need only a vector of MovieLibrary objects, nothing more, but there is not enough information in the question to tell.

  •  Tags:  
  • Related