Search code examples
c++heap-memorydelete-operatorheap-corruption

delete[] causing heap corruption


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.


Solution

  • The code is now working.

    It was wrong all over. I messed up pretty much any part that I could regarding the dynamic memory while trying to fix it before.

    I initially didn't care about calling 3 times to strlen becuase it's just homework and a very small program but I guess it's better to get used to do things the right way... I also dropped the copy which I evidently don't understand very well in favour of a simple for loop.

    // Add function. The rest is cut.
        int word_length = strlen(word);
    
        temp[i] = new char[word_length + 1];    // Added +1 here.
        temp[i][word_length] = '\0';    /* This was correct after all.
        the word_length index is the correct ending.*/
    
        for (int j = 0; j < word_length; j++)   // copy replaced by for loop.
            temp[i][j] = word[j];
        // cut
    }
    
    void Delete_Word(char**& dictionary, int& dictionary_size, char* word)
    {
        delete[] Search_For_Word(dictionary, dictionary_size, word);
       // There was a -1 mistake here I made in order to try and fix the thing earlier.
    // No need for more, it works perfectly now.