C++ errors with releasing Dynamic memory

Question

Error - *** error for object 0x7ffeefbff360: pointer being freed was not allocated.

I understand its better to use vectors, but our prof wants us to use it this way.

My unwrap function is giving me errors where I want to release the memory, also when I want to print the pattern using the for loop in the display function it gives me a garbage value out of memory instead of printing out the pattern itself. I tested using cout in the wrap function and it works there but not in my display function.

bool wrap(Gift& theGift){

    if (theGift.m_wrap == nullptr) {
        cout << "Wrapping gifts..." << endl;

        do {
            cout << "Enter the number of wrapping layers for the Gift: ";
            cin >> theGift.m_wrapLayers;
        }while ((theGift.m_wrapLayers <= 0) && cout << "Layers at minimum must be 1, try again." << endl);

        theGift.m_wrap = new Wrapping[theGift.m_wrapLayers];
        char buffer[100];
        int patternLength;

        for (int i = 0; i < theGift.m_wrapLayers; i++) {
            cout << "Enter wrapping pattern #" << i + 1 << ": ";
            cin >> buffer;
            patternLength = (unsigned)strlen(buffer);
            theGift.m_wrap[i].m_pattern = new char[patternLength + 1];
            theGift.m_wrap[i].m_pattern = buffer;
            cout << theGift.m_wrap[i].m_pattern << endl;
        }
        return true;

    }else {
        cout << "Gift is already wrapped!" << endl;
        return false;
    }
}

bool unwrap(Gift& theGift){
    if (theGift.m_wrap != nullptr) {
        cout << "Gift being unwrapped." << endl;
        for (int i = 0; i < theGift.m_wrapLayers; i++) {
            delete[] theGift.m_wrap[i].m_pattern;
            theGift.m_wrap[i].m_pattern = nullptr;
        }
        delete[] theGift.m_wrap;
        theGift.m_wrap = nullptr;

        return true;
    }else{
        cout << "Gift isn't wrapped! Cannot unwrap." << endl;
        return false;
    }
}

void display(Gift& theGift){
        cout << "Gift Details: " << endl;
        cout << " Description: " << theGift.m_description << endl;
        cout << "       Price: " << theGift.m_price << endl;
        cout << "       Units: " << theGift.m_units << endl;
        cout << "Wrap Layers: " << theGift.m_wrapLayers << endl;
        for (int i = 0 ; i < theGift.m_wrapLayers; i++) {
            cout << "Wrap #" << i + 1 << " ->" << theGift.m_wrap[i].m_pattern << endl;

    }

}

Does `Gift` correctly implement copy constructor and copy assignment, as per the [Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three)? — Mooing Duck, Jun 05 '20 at 18:31
Please provide a [Minimal, Reproducable Example](https://stackoverflow.com/help/minimal-reproducible-example) so that we can accurately answer the question. — Mooing Duck, Jun 05 '20 at 18:32
Does this answer your question? [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — drescherjm, Jun 05 '20 at 18:39
your problems comes from the line `theGift.m_wrap[i].m_pattern = buffer;` in *wrap*, see my answer — bruno, Jun 05 '20 at 18:42
@bruno, yes thank you it fixed it. The only thing left is I am allocating memory here theGift.m_wrap[i].m_pattern = new char[patternLength + 1]; how should i go about releasing it. — CuzWhyNot Vlogs, Jun 05 '20 at 18:44
@CuzWhyNotVlogs replacing `theGift.m_wrap[i].m_pattern = buffer;` by `strcpy(theGift.m_wrap[i].m_pattern, buffer);` the rest of the code is a priori correct, including `delete[] theGift.m_wrap[i].m_pattern;`. Just the line `theGift.m_wrap[i].m_pattern = nullptr;` is useless because after you do `delete[] theGift.m_wrap;` — bruno, Jun 05 '20 at 18:46
As other hint, replace `void display(Gift& theGift){` by `void display(const Gift& theGift){` to clealy indicate you do not want to modify the argument, and to see the error the compiler produces in case you do. The more you restrict your code the better it is — bruno, Jun 05 '20 at 18:49

bruno · Accepted Answer · 2020-06-05T18:57:15.230

Error - *** error for object 0x7ffeefbff360: pointer being freed was not allocated.

in wrap :

char buffer[100];
...
theGift.m_wrap[i].m_pattern = buffer;

you save in the in-out parameter theGift a pointer to the local array buffer (and you have a memory leak for the lost of the allocation done in theGift.m_wrap[i].m_pattern = new char[patternLength + 1]; just before)

and later in unwrap :

delete[] theGift.m_wrap[i].m_pattern;

you try to delete that invalid pointer.

In fact in wrap rather than :

theGift.m_wrap[i].m_pattern = new char[patternLength + 1];
theGift.m_wrap[i].m_pattern = buffer;

you wanted to do :

 theGift.m_wrap[i].m_pattern = new char[patternLength + 1];
 strcpy(theGift.m_wrap[i].m_pattern, buffer);

Note you can also use std::string for m_pattern rather than array of char, and a std::vector<Wrapping>for m_wrap rather than an array of Wrapping. That simplify a lot, no new nor delete

Given the code, the poster might not be clear as to what the bug is here. They're probably assuming this copies the pointed data. So: You're right, but can you explain it clearer? — Mooing Duck, Jun 05 '20 at 18:33
@MooingDuck the error is `Error - *** error for object 0x7ffeefbff360: pointer being freed was not allocated.`, this is what the code I extracted does — bruno, Jun 05 '20 at 18:34

C++ errors with releasing Dynamic memory

1 Answers1