0

I am attempting to build a dictionary using C++. The dictionary must be dynamically created and updated at all times. For example, say I have 5 words in my dictionary and I want to add another word, I must create a new dictionary with room for 6 words, copy the old words and add the new word to the new dictionary.

In my main function, I created a lexicon** (pointer to an array of pointers, since each word has a char pointer to it). I have created a newStr function to receive the new word and to add it to the dictionary and to resort it alphabetically.

The program runs once, but when I want to add another word I get an access violation warning:

0xC0000005: Access violation reading location 0xDDDDDDDD.

I don't understand what I am doing wrong. Thanks for the help!

Here's my code:

#define MAX 80
#include <iostream>
#include <cstring>
#include <string.h>
using namespace std;

void newStr(char** lexicon, int& lexiconSize, char word[])
{
    // create the new updated lexicon
    char** updated = new char*[++lexiconSize];

    // copy the words from the old to the updated lexicon
    for (int i = 0; i < lexiconSize; i++)
    {
        updated[i] = new char[MAX];

        if (i < lexiconSize - 1)
        {
            strcpy_s(updated[i], MAX, lexicon[i]);
        }

        // add the new word to the end of the updatedLexicon
        else
        {
            strcpy_s(updated[i], MAX, word);
        }
    }

    // deallocate the memory of the worlds of the old lexicon
    for (int i = 0; i < lexiconSize - 1; i++)
    {
        delete[] lexicon[i];
    }

    // deallocate the memory of the old lexicon
    delete[] lexicon;

    // point the lexicon pointer to the updatedLexicon
    lexicon = updated;

    // now sort the lexicon including the new word
    if (lexiconSize > 1)
    {
        for (int i = 1; i < lexiconSize; i++)
        {
            for (int j = 1; j < lexiconSize; j++)
            {
                if (strcmp(lexicon[j - 1], lexicon[j]) > 0)
                {
                    char t[MAX];
                    strcpy_s(t, MAX, lexicon[j - 1]);
                    strcpy_s(lexicon[j - 1], MAX, lexicon[j]);
                    strcpy_s(lexicon[j], MAX, t);
                }
            }
        }
    }

    // deallocate the memory created for the updated lexicon
    for (int i = 0; i < lexiconSize; i++)
    {
        delete[] updated[i];
    }

    delete[] updated;

    return;
}

int main()
{
    int lexiconSize = 3;
    char** lexicon;
    char word[MAX] = {};

    // initialize lexicon for testing:
    lexicon = new char*[lexiconSize];
    lexicon[0] = new char[MAX];
    strcpy_s(lexicon[0], MAX, "maybe");
    lexicon[1] = new char[MAX];
    strcpy_s(lexicon[1], MAX, "this");
    lexicon[2] = new char[MAX];
    strcpy_s(lexicon[2], MAX, "works");

    cout << "enter the word to add" << endl;
    cin >> word;

    newStr(lexicon, lexiconSize, word);

    // menu system that allows to add/delete/print the words

    // delete the lexicon at the end of the program
    for (int i = 0; i < lexiconSize; i++)
    { // delete the internal words
        if (lexicon[i])
        {
            delete[] lexicon[i];
        }
    }

    if (lexicon)
    {
        delete[] lexicon;
    }

    return 0;
}
Azeem
  • 11,148
  • 4
  • 27
  • 40
Tal J
  • 27
  • 4
  • 1
    Your biggest mistake is not using [`std::vector`](http://en.cppreference.com/w/cpp/container/vector). – StoryTeller - Unslander Monica Jan 14 '18 at 10:24
  • And even if you must use raw pointers there is no reason to not copy the pointers from the existing array to the new, if you do that only the existing array (and not the contained pointer values) need be deleted during an insert. – SoronelHaetir Jan 14 '18 at 10:29
  • Must use pointers. I'll try by just copying the pointers, thanks. – Tal J Jan 14 '18 at 10:49
  • 1
    Dictionary? `std::map` –  Jan 14 '18 at 10:56
  • @TalJ: `std::map` and `std::string` internally use pointers for you. – Christian Hackl Jan 14 '18 at 10:57
  • I must be missing something. During de-bugging I can see the new lexicon is being created properly. The problem arises when I de-allocate the memory. Is my structure correct?: 1) Create a new lexicon with the updated size 2) Copy the pointers from the old lexicon to the new one 3) Add the new word to the end of the new lexicon (by creating a new char[MAX] Until this point I can see that everything works fine, and I can even sort this lexicon and print out the words. How must I deal with the memory from this point on-wards? What must be de-allocated and what must not? Thanks – Tal J Jan 14 '18 at 11:02
  • Why must you use raw pointers? Why even allocate anything if the size is fixed? – Passer By Jan 14 '18 at 11:32
  • @PasserBy The size is not fixed. The lexicon must change it's size according to the number of words. I can add or remove words. – Tal J Jan 14 '18 at 11:57

2 Answers2

1

Your problem is that lexicon is passed to newStr() by value.

The assignment lexicon = updated is therefore not visible to the caller.

Since the function releases all dynamically allocated memory referenced by lexicon[i], all subsequent uses of lexicon in main() therefore have undefined behaviour.

Incidentally, all the memory allocated inside newStr() is leaked - no variable refers to it after the function returns, so it cannot be released in code.

Instead of trying to use pointers and operator new directly, look up standard containers (std::vector) and std::string (to manage string data).

Peter
  • 35,646
  • 4
  • 32
  • 74
  • If I understand correctly, lexicon is being passed by reference since it is a pointer. If not, how would I pass it by reference and not by value? I must use pointers. – Tal J Jan 14 '18 at 10:51
  • You understand incorrectly. `lexicon` is a pointer (a variable who's value is the address of a variable) and it is being passed by value. `*lexicon` and `**lexicon` (since `lexicon` is a pointer to a pointer) can be used as references. – Peter Jan 14 '18 at 12:44
0
  • This is not a C++ code: it's C with some bit of syntax sugar.
  • Do not deallocate something inside function, if you have created it in another place.
  • Use smart pointers.
  • Do you really think, that your implementation will be more effective and potable, than std::vector/std::map?
A.N.
  • 278
  • 2
  • 13
  • Unfortunately I must use pointers for this. Regarding the deallocation of the memory inside the function, how would I recreate the lexicon without deallocation? – Tal J Jan 14 '18 at 10:53
  • Use classes to incapsulate `lexicon`. – A.N. Jan 14 '18 at 10:57
  • could you please further explain what you mean? Thanks @A.N. – Tal J Jan 14 '18 at 11:58
  • If you write in C++, you need to use another design, not like in C. Use class to implement `lexicon`. Make class with a private variable for the `lexicon` storage, make storage allocator method, storage deallocator method. After this, write reallocation method, using allocator+copy+deallocator (then, you can optimize it, if it's really need). Word appending method will be simple to write. – A.N. Jan 14 '18 at 12:18