Memory Allocation - Using delete properly

Question

All my program crashes because of delete [] meanings;, delete [] meanings;, delete [] temp_meaning; , when I remove these 3 lines it works fine, so probably I am using the delete wrongly ... can anybody enlighten me here please?

#include <iostream>
#include <string>
#include <cstring>
using namespace std;

class Expression {

    char *word_with_several_meanings; // like "bank", "class"
    char **meanings; // a pointer to a pointer stores all meanings
    int meanings_ctr; // meanings counter

    //-----------FUNCTIONS------------------------------------------------
public:
    void word(const char* = NULL );
    void add_meaning(char * = NULL);
    char* get_word();
    int get_total_number_of_meanings();
    char* get_meaning(int meanx = 0);
    Expression(int mctr = 0); // CTOR
    ~Expression(); // DTOR
};

Expression::Expression(int mctr ) {
    meanings_ctr = mctr;    // Setting the counter to 0
    meanings = new char * [meanings_ctr]; // Allocate Space for meanings
}

Expression::~Expression() {

    while(meanings_ctr-->0){
    delete meanings[meanings_ctr];
    }
    delete [] meanings; // Deleting the memory we allocated
    delete [] word_with_several_meanings; // Deleting the memory we allocated
}

void Expression::word(const char *p2c )
{

    word_with_several_meanings = new char[strlen(p2c)+1];
    // copy the string, DEEP copy
    strcpy(word_with_several_meanings, p2c);
}

void Expression::add_meaning( char  * p2c)
{

    //meanings[ meanings_ctr ] = new char [strlen(p2c) + 1];
    //strcpy(meanings[ meanings_ctr++ ] , p2c);
    // temp 
    if (meanings_ctr < 1){
    meanings[ meanings_ctr ] = new char [strlen(p2c) + 1];
    strcpy(meanings[ meanings_ctr++ ] , p2c);
    }
    else {
int temp_ctr;
    char **temp_meaning;
    temp_meaning = new char * [meanings_ctr-1];
    for(temp_ctr =0; temp_ctr<meanings_ctr;temp_ctr++){
        temp_meaning[temp_ctr] = new char [strlen(meanings[ temp_ctr ]) + 1];
            strcpy(temp_meaning[temp_ctr], meanings[ temp_ctr ]);
    }
    for (temp_ctr =0; temp_ctr<meanings_ctr;temp_ctr++){
            delete meanings[temp_ctr];

    }
    delete [] meanings;

    meanings = new char * [meanings_ctr];
    for(temp_ctr =0; temp_ctr<meanings_ctr;temp_ctr++){
        meanings[ temp_ctr ] = new char [strlen(temp_meaning[temp_ctr]) + 1];
            strcpy(meanings[ temp_ctr ], temp_meaning[temp_ctr]);
    }
    meanings[ meanings_ctr ] = new char [strlen(p2c) + 1];
    strcpy(meanings[ meanings_ctr ] , p2c);
            for (temp_ctr =0; temp_ctr<meanings_ctr;temp_ctr++){
            delete temp_meaning[temp_ctr];
    }
    delete [] temp_meaning;
            meanings_ctr++;
    }


}

char * Expression::get_meaning( int meanx )
{

    return *(meanings+meanx);

}

char * Expression::get_word()
{

    return word_with_several_meanings;

}

int Expression::get_total_number_of_meanings()
{
    return meanings_ctr;
}


int main(void) {
    int i;
    Expression expr;
    expr.word("bank");
    expr.add_meaning("a place to get money from");
    expr.add_meaning("b place to sit");
    expr.add_meaning("4 letter word");
    expr.add_meaning("Test meaning");
    cout << expr.get_word() << endl;

    for(int i = 0; i<expr.get_total_number_of_meanings(); i++)
            cout << " " << expr.get_meaning(i)  << endl;
    Expression expr2;
    expr2.word("class");
    expr2.add_meaning("a school class");
    expr2.add_meaning("a classification for a hotel");
    expr2.add_meaning("Starts with C");
    cout << expr2.get_word() << endl;
    for( i = 0; i<expr2.get_total_number_of_meanings(); i++)
            cout << " " << expr2.get_meaning(i) << endl;

    Expression expr3;
    expr3.word("A very long test");
    char str[] = "Meaning_    ";
    for(int kx =0; kx<31; kx++){
            str[8] = ('A'+kx);
            expr3.add_meaning(str);
    }

    cout << expr3.get_word() << endl;
    for( int i = 0; i<expr3.get_total_number_of_meanings(); i++)
            cout << " " << expr3.get_meaning(i) << endl;
    return 0;
}

Answer 1

This program exhibits signs of memory corruption that comes from the following statements:

    meanings_ctr = mctr;    // Setting the counter to 0
    meanings = new char * [meanings_ctr]; // Allocate Space for meanings

and

    meanings = new char * [meanings_ctr];

Since add_meaning() contains the following code:

if (meanings_ctr < 1){
   meanings[ meanings_ctr ] = new char [strlen(p2c) + 1];

you actually write at meanings[0], while you allocated 0 bytes for it. Since indices in C start from 0 for an array with the highest index at max_index you need to allocate max_index+1 elements. For an array with max_index = 0 you need to allocate 1 element.

In other words you need to allocate meanings = new char * [meanings_ctr + 1] instead of new char * [meanings_ctr], and temp_meaning = new char * [meanings_ctr] instead of new char * [meanings_ctr - 1].

As for the use of delete and delete[] the general rule is that what was allocated with new should be deallocated with delete and what was allocated with new[] should be destroyed with delete[]. There is a thread on it: delete vs delete[] operators in C++. And some good background can be found in the answers in how does delete know it is an array.

Here is how to debug the program without using any expensive tools or tools that are difficult to learn.

If you add debug prints into your constructor and destructor like this:

Expression::Expression(int mctr ) {
    meanings_ctr = mctr;    // Setting the counter to 0
    meanings = new char * [meanings_ctr]; // Allocate Space for meanings
    cout << "[debug] allocated " << sizeof(char*)*meanings_ctr << " bytes @" << 
             hex << meanings << dec << endl;
}

And

Expression::~Expression() {
    while(meanings_ctr-- > 0){
//       if(meanings[meanings_ctr]) delete [] (meanings[meanings_ctr]);
    }
    cout << "[debug] to deallocate @" << hex << meanings << dec << endl;
//   delete [] meanings; // Deleting the memory we allocated
//    delete [] word_with_several_meanings; // Deleting the memory we allocated
}

and similarly in add_meaning(), you get

[debug] allocated 0 bytes @0x804c008
[debug] to deallocate @0x804c008
[debug] allocated 4 bytes @0x804c078
 ...
[debug] allocated 120 bytes @0x804fa78
[debug] to deallocate @0x804fa78
[debug] to deallocate @0x804c260
[debug] to deallocate @0x804c150

What looks worrying here is allocated 0 bytes. As the code in add_meanings() contains:

if (meanings_ctr < 1){
meanings[ meanings_ctr ] = new char [strlen(p2c) + 1];

it uses memory @ meanings[0] that was not allocated and leads to corruption.

For reference here are all the accumulated changes:

25c25
<     meanings = new char * [meanings_ctr]; // Allocate Space for meanings
---
>     meanings = new char * [meanings_ctr + 1]; // Allocate Space for meanings
30,31c30,31
<     while(meanings_ctr-->0){
<     delete meanings[meanings_ctr];
---
>     while(meanings_ctr-- > 0){
>         delete [] meanings[meanings_ctr];
58c58
<     temp_meaning = new char * [meanings_ctr-1];
---
>     temp_meaning = new char * [meanings_ctr ];
64c64
<             delete meanings[temp_ctr];
---
>          delete [] meanings[temp_ctr];
69c69
<     meanings = new char * [meanings_ctr];
---
>     meanings = new char * [meanings_ctr + 1];
76,77c76,77
<             for (temp_ctr =0; temp_ctr<meanings_ctr;temp_ctr++){
<             delete temp_meaning[temp_ctr];
---
>     for (temp_ctr =0; temp_ctr<meanings_ctr;temp_ctr++){
>            delete [] temp_meaning[temp_ctr];

Answer 2

You must come from the C world, you wouldn't find any char* in a C++ program to store a word... you should have a look at std::string, it will enlighten your day I can assure you.

By the way in C++ we tend to get rid of all those delete and delete [], you should maybe pick a good C++ book and learn proper C++, not a C with classes that has been used 30 years before.