Exception thrown at 0x0037A5C2 project.exe: 0xC0000005: Access violation writing location 0xDDDDDDDD at the end of the program

1

I have encountered this runtime exception at the very end of the program by simply creating an instance of the specified class, so I presume the issue lies with either the constructor, copy constructor, copy assignment operator or destructor. I have read up on and followed the rule of three to the extent of my limited cpp knowledge.

Source.cpp

#include "Header.h"
#include <iostream>

using namespace std;


int main() {


string command = "CREATE TABLE table_name IF NOT EXISTS ((column_1_name,type,default_value), (column_2_name,type,default_value))";
string columns[20] = { "column_1_name,type,default_value", "column_1_name,type,default_value" };
string commandData[9] = { "table_name", "IF NOT EXISTS" };

CommCREATETABLE comm(command, columns, commandData, 2, 2);

}

Relevant code from Header.h

class CommCREATETABLE {
    string fullCommand = "";
    string* columns = nullptr;
    string* commandData = nullptr;
    string tableName = "";
    int nrOfColumns = 0;
    int nrOfElements = 0;
    bool valid = false;

Constructor:

CommCREATETABLE(string command, string* columns, string* commandData, int nrOfRows, int nrOfElements) {
        this->setNrOfColumns(nrOfRows);
        this->setNrOfElements(nrOfElements);
        this->setCommand(command);
        this->setColumns(columns);
        this->setCommandData(commandData);
        this->valid = checkInput(this->commandData, this->columns);
        this->setTableName(commandData[0]);
    }

Copy constructor, copy assignment operator, destructor:

CommCREATETABLE(const CommCREATETABLE& comm) {
        this->setNrOfColumns(comm.nrOfColumns);
        this->setNrOfElements(comm.nrOfElements);
        this->setCommand(comm.fullCommand);
        this->setColumns(comm.columns);
        this->setCommandData(comm.commandData);
        this->setTableName(comm.tableName);
        this->valid = comm.valid;
    }
    ~CommCREATETABLE() {
        if (this->columns != nullptr) {
            delete[] this->columns;
        }
        if (this->commandData != nullptr) {
            delete[] this->commandData;
        }
    }

    CommCREATETABLE& operator=(const CommCREATETABLE& comm) {
        this->setCommand(comm.fullCommand);
        this->setColumns(comm.columns);
        this->setCommandData(comm.commandData);
        this->setTableName(comm.tableName);
        this->setNrOfColumns(comm.nrOfColumns);
        this->setNrOfElements(comm.nrOfElements);
        this->valid = checkInput(this->commandData, this->columns);
        return *this;
    }

The only setters that deal with dynamic memory allocation are the following:

void setColumns(const string* columns) {
        if (this->nrOfColumns >= 0) {
            this->columns = new string[this->nrOfColumns];
            memcpy(this->columns, columns, this->nrOfColumns * sizeof(string));
        }
        else throw EmptyCommandException();
    }
    void setCommandData(const string* commandData) {
        if (this->nrOfElements >= 0) {
            this->commandData = new string[this->nrOfElements];
            memcpy(this->commandData, commandData, this->nrOfElements * sizeof(string));
        }
        else throw EmptyCommandException();
    }
c++
constructor
destructor
copy-constructor
memcpy
asked on Stack Overflow Nov 23, 2020 by Polly • edited Nov 23, 2020 by Polly

1 Answer

2

At a quick glance I would say the issue is in your setColumns and setCommandData functions. (I might of course be wrong, I did not try to run the code you presented nor the changes I made -- so there might also be a typo somewhere.)

There you use memcpy to copy the strings into your class. However, internally a C++ string holds a pointer to the actual string, so using memcpy actually only copies that pointer. As a result, once the original string gets deleted, the pointer you copied into your class is no longer valid (as the memory has already been freed). As a result, once your class also gets deleted it attempts to delete memory that has already been freed. That is probably where your error comes from.

In fact, if you added lines to your program where you tried to manipulate your class (after the original input strings have already been deleted), the problem would present itself even sooner, as you would be accessing memory that has already been freed. This would lead to undefined behaviour, which typically ends with a crash at some point.

A quick fix would be to change the way you copy the data, by using = for each string (in that way copying the actual strings into a new location in memory, rather than copying the pointer).

void setColumns(const string* columns) {
    if (this->nrOfColumns > 0) { // Creating an array of size 0 is also not a good idea. 
        this->columns = new string[this->nrOfColumns];
        for (int i = 0; i < nrOfColumns; i++) { // You don't need this everywhere. 
            this->columns[i] = columns[i]; 
            // I don't think naming things the exact same way is good practice. 
        }
    }
    else throw EmptyCommandException();
}
void setCommandData(const string* commandData) {
    if (this->nrOfElements > 0) { // Creating an array of size 0 is also not a good idea. 
        this->commandData = new string[this->nrOfElements];
        for (int i = 0; i < nrOfElements; i++) { // You don't need this everywhere. 
            this->commandData[i] = commandData[i]; 
            // I don't think naming things the exact same way is good practice. 
        }
    }
    else throw EmptyCommandException();
}

Alternatively, if you want to avoid making copies you should look into move, but I would suggest against this for the time being, if you are still learning. You'll get there soon enough.

answered on Stack Overflow Nov 23, 2020 by Qubit

User contributions licensed under CC BY-SA 3.0