delete[] causing heap corruption

Question

I'm well aware that there are countless problems like this, but I searched for hours and couldn't understand what I did wrong so I would really appreciate your help. (I'm new to programming)

I need to create a dictionary manager of sorts as part of my homework but I seem to have a problem with deleting words. I get an error message "...triggered a breakpoint".

The usual answer people get to this problem is that this is heap corruption caused by going out of bounds but I can't see if and how I caused this.

I already made something similar with bus info management and it worked perfectly so that makes me even more confused... (Obviously, I did not make the mechanism exactly the same, but even after looking at my previous code I couldn't isolate the problem)

I added the functions I believe are of concern,

The adding function:

void Add_Word(char**& dictionary, int& dictionary_size, char word[])
{
    char** temp = new char*[dictionary_size + 1];   // Create a new array of appropriate size.

    int i;
    for (i = 0; i < dictionary_size; i++)
    {
        temp[i] = dictionary[i];    // Copy head pointers addresses for all existing items.
    }
    temp[i] = new char[strlen(word)];   // Add the space for the new word,
    temp[i][strlen(word)] = '\0';   // mark its end

    strcpy_s(temp[i], strlen(word) + 1, word);  // then copy it.
    // I'm really not so sure about what I should put in the buffer length but
    // strlen(word) + 1 seemed to work... I know... not good, but strlen(word) alone caused a problem.

    if (dictionary_size > 0)
        delete []dictionary;    // Delete previous head pointers array if there are any and
    dictionary = temp;  // reset the main pointer to the address of the new one.

    dictionary_size++;  // Finally, increase dictionary_size.
}

The deleting function:

void Delete_Word(char**& dictionary, int& dictionary_size, char* word)
{
    // !!! This is where the crash thingy happens.
    delete[] Search_For_Word(dictionary, dictionary_size, word);    // Delete the word from the dictionary.
    // Search_For_Word returns a pointer to the word it receives, from the dictionary.

    char** temp = new char*[dictionary_size - 1];   // Create a new array of appropriate size.

    int i;
    for (i = 0; i < dictionary_size; i++)
    {
        if (dictionary[i][0])
            temp[i] = dictionary[i];    // Copy the head pointers of the existing
           // items to the new array except for the deleted word.
    }

    delete[] dictionary;    // Delete previous head pointers array and
    dictionary = temp;  // reset the main pointer to the address of the new one.

    dictionary_size--;  // Finally, decrease dictionary_size.
}

EDIT: Any parts that are excessively inefficient or obviously broken are likely a result of me messing with my code trying to figure this out on my own (such as the calling 3 times to strlen mentioned (thanks again for that, kfsone...), or forgetting to +1 it for the '\0' to mark the end of a string --actually, no, if we go by obvious you won't tell me my mistakes @.@).

As for the reason I'm dealing with char instead of strings and vectors please allow me to quote myself: "...as part of my homework". I just barely started programming. That, and I want to grasp the basics before moving on to using the more comfortable higher-up tools.

Answer 1

Change:

temp[i] = new char[strlen(word)]

To:

temp[i] = new char[strlen(word)+1]

Answer 2

Your code has several problems.

First, if you want to allocate a C-style string on the heap using new[], then you must pay attention to the terminating NUL character.

So, if you want to do a deep copy from a string word, then you must calculate enough room, considering strlen(word) + 1: the +1 is for the terminating NUL character.
e.g.:

// Original code (wrong):
//
//     temp[i] = new char[strlen(word)];
//
// New code:
temp[i] = new char[strlen(word) + 1]; // consider terminating NUL (+1)

Moreover, following your code with explicit new[]s and delete[]s is not easy.
In modern C++, you may want to use convenient robust container classes like std::vector and string classes like std::string, instead of raw C-style pointers and strings.

You can simply store a list of strings using a std::vector<std::string>, and vector::push_back() method to add new strings to the vector. No need to complicate code with new[], delete[], strcpy_s(), etc.

And if you want to deep-copy strings, you can just use the simple natural overload of operator= for std::string, and copy constructors; e.g. std::string temp = word; will work just fine.