error Debug Assertion Failed - when deleting a dynamically allocated array

Question

I've been trying to complete a class assignment but I keep getting an error that I can't seem to resolve. Debug Assertion failed I've narrowed the problem (I think) to the destructor. If I comment out the line delete[] english; there is no error. I've tried reading through other threads, but they haven't helped solve this. Here are the three files in the project (I took out all the rest of the code because the error still occurs with just this):

Here is the header file:

//Dictionaty.h
#ifndef DICTIONARY_H
#define DICTIONARY_H

class Dictionary
{
public:
    Dictionary();
    ~Dictionary();

   void setEnglish(char*);
   char* getEnglish();


private:
    char *english;

};

#endif

Here is where the functions are:

//dictionary.cpp
#include <iostream>
#include "Dictionary.h"

using namespace std;

Dictionary::Dictionary()
{
    english = new char[20];


    strcpy(english, " ");
    //strcpy(french, " ");
}

Dictionary::~Dictionary()
{
    delete[] english; // problem is here (i think)

}

void Dictionary::setEnglish(char *eng)
{
    english = eng;
    cout << "the value of english in \"setEnglish()\" is: " << english << endl; //db
}

and this is the driver:

//dictionaryDrv.cpp
#include <iostream>
#include "Dictionary.h"

using namespace std;

int main()
{
    Dictionary words[30];

    //readIn(words);
    char test[5] = "test";
    words[0].setEnglish(test);


    system("pause");
    return 0;
}

Answer 1

The problem is

char test[5] = "test";
words[0].setEnglish(test);

Then the member varialbe english is assigned to the pointer decayed from array test, which isn't dynamic allocated using new[] and then can't be delete[]ed, but it's exactly what the destructor is trying to do. Therefore UB.

According to the intent of your code, you should use strcpy or strncpy in Dictionary::setEnglish, don't assign the pointer directly.

Other suggestions:

1. Consider about The Rule of Three, especially when you use raw pointers (such as char*).

2. Use std::string instead of C-style strings (char*).

Answer 2

`char test[5] = "test";  words[0].setEnglish(test);`

Here test is allocated in the stack (there is no new there). Then english = eng; will direct the pointer english toward it, you cannot delete from the stack.

the fix: since you want your object to own the string, you should copy it.

void Dictionary::setEnglish(char *eng)
{
    delete[] english;
    english = new char[strlen(eng) + 1];
    strcpy(english, eng);
    cout << "the value of english in \"setEnglish()\" is: " << english << endl; //db
}

Finally, better use std::string and avoid lot of head-aches.

Answer 3

There are multiple bugs in the shown code.

english = new char;

This sets english to a dynamically-allocated array of one character, then immediately afterward:

strcpy(english, " ");

This copies two chars - a space and a \0 - into a buffer of only one character. This runs off at the end of the array, corrupting memory, and results in undefined behavior.

Furthermore, in the destructor:

 delete[] english; 

That, by itself, is fine. Except:

void Dictionary::setEnglish(char *eng)
{
    english = eng;

As a result of this being called from main(), english gets set to a pointer to a buffer that was not allocated in dynamic scope. As a result, the destructor will attempt to delete something that was not newed. This also results in undefined behavior.

Furthermore, looking ahead, the shown class violates the Rule of 3, and, as such, it's quite easily to use it incorrectly, resulting in further bugs.

In conclusion, the shown code does not properly handle dynamically-allocated memory, committing several errors along the ones, from overwriting non-allocated memory, to deleteing memory that was never newed in the first place.

You need to reread and study the chapter in your C++ book that talks about how to properly use dynamically-allocated memory.

Answer 4

The issue is how you set the value inside setEnglish method, the assigment of english = eng don't allocate the value in right way so, It can be fixed if you use strcpy instead:

Use:

    void Dictionary::setEnglish(char *eng) {
        strcpy(english,eng);
        cout << "the value of english in \"setEnglish()\" is: " << english << endl; 
}

you will get the right behaviour