1

I compiled my code, but it threw me "Exception thrown: read access violation. _First was nullptr."

I literally have no idea what this means as I'm still a beginner in C++. I really need your help to figure this problem as I've been struggling with this code for days, and it's so frustrating...

Thank you in advance.

#include <iostream>
#include <cstring>
#include <fstream>
using namespace std;

class MyString {
public:

    //default constructor
    MyString();

    MyString(char* chars);

    //copy constructor
    MyString(const MyString &);

    int length() const;

    //destructor
    ~MyString();

    //operator overloads
    char& operator[](int index);
    friend MyString operator+(const MyString& newWord, const MyString& newWord2);
    MyString& operator+=(const MyString& newWord);
    friend ostream& operator<<(ostream& newWord, const MyString& newWord2);
    friend istream& operator >> (istream& newWord, MyString& newWord2);
    friend bool operator==(const MyString& newWord, const MyString& newWord2);
    friend bool operator!=(const MyString& newWord, const MyString& newWord2);
    friend bool operator<(const MyString& newWord, const MyString& newWord2);
    friend bool operator<=(const MyString& newWord, const MyString& newWord2);
    friend bool operator>(const MyString& newWord, const MyString& newWord2);
    friend bool operator>=(const MyString& newWord, const MyString& newWord2);

private:
    char* value;
    int size;
};

//default constructor
MyString::MyString() {

    value = NULL;
    size = 0;
}

//copy constructor
MyString::MyString(const MyString& newWord) {

    //perform a deep copy to copy each of the value to a new memory
    size = newWord.size;
    char* newCopy = new char[size];

    for (int ii = 0; ii < size; ii++) {
        newCopy[ii] = newWord.value[ii];
    }
}

//constructor with an argument
MyString::MyString(char* chars) {

    //give the value and the size
    value = chars;
    size = strlen(chars);
}

//find length
int MyString::length() const {

    return size;
}

//find the value of each index
char& MyString::operator[](int index) {

    return value[index];
}

//operator + (concatenate)
MyString operator+(const MyString& newWord, const MyString& newWord2) {

    MyString concatenated;
    concatenated = strcat(newWord.value, newWord.value);
    return concatenated;

}

//operator += (append)
MyString& MyString::operator+=(const MyString& newWord) {

    char * newMemory = value;
    value = new char[strlen(value) + newWord.length() + 1];
    strcpy(value, newMemory);
    strcat(value, newWord.value);
    if (size != 0)
    {
        delete[] newMemory;
    }
    size = strlen(value);
    return *this;
}

//ostream operator
ostream& operator<<(ostream& newWord, const MyString& newWord2) {

    newWord << newWord2.value;
    return newWord;
}


//istream operator
istream& operator >> (istream& newWord, MyString& newWord2) {

    const int MAX = 100;
    char* ptr = new char[MAX];
    newWord >> ptr;
    newWord2 = MyString(ptr);
    delete ptr;
    return newWord;
}

//all boolean operators
bool operator==(const MyString& newWord, const MyString& newWord2) {
    if (newWord.value == newWord2.value) {
        return true;
    }
    else {
        return false;
    }
}

bool operator!=(const MyString& newWord, const MyString& newWord2) {
    if (newWord.value != newWord2.value) {
        return true;
    }
    else {
        return false;
    }
}

bool operator<(const MyString& newWord, const MyString& newWord2) {
    if (newWord.value < newWord2.value) {
        return true;
    }
    else {
        return false;
    }
}

bool operator<=(const MyString& newWord, const MyString& newWord2) {
    if (newWord.value <= newWord2.value) {
        return true;
    }
    else {
        return false;
    }
}

bool operator>(const MyString& newWord, const MyString& newWord2) {
    if (newWord.value > newWord2.value) {
        return true;
    }
    else {
        return false;
    }
}

bool operator>=(const MyString& newWord, const MyString& newWord2) {
    if (newWord.value >= newWord2.value) {
        return true;
    }
    else {
        return false;
    }
}

//destructor to release memory
MyString::~MyString() {
    delete[] value;
}

void test_copy_and_destructor(MyString S) {
    cout << "test: copy constructor and destructor calls: " << endl;
    MyString temp = S;
    cout << "temp inside function test_copy_and_destructor: " << temp << endl;
}

int main() {

    MyString st1("abc abc");
    MyString st2("9fgth");

    cout << "Copy constructor , << operator" << endl;

    MyString  st3(st1);

    cout << "st3: " << st3 << endl;

    test_copy_and_destructor(st2);

    MyString  st4;

    cout << "operator + " << endl;

    st4 = st3 + st2;

    cout << "st4: " << st4 << endl;

    cout << "st1 + st2: " << (st1 + st2) << endl;

    cout << "operators  [ ] " << endl;

    for (int i = 0; i < st2.length(); i++)
        cout << st2[i] << " ";

    cout << endl;

    cout << "operators  += , ==, != " << endl;

    st2 += st1;

    if (st3 == st1)
        cout << "st3 and st1 are identical " << endl;
    else cout << "st3 and st1 are not identical " << endl;

    if (st2 != st1)
        cout << "st2 and st1 are not identical " << endl;
    else cout << "st2 and st1 are identical " << endl;

    cout << "operators  < , <=, >, >= " << endl;

    if (st2 < st1)
        cout << "st2 < st1 " << endl;
    else cout << "st2 is not less than st1 " << endl;

    if (st1 <= st2)
        cout << "st1 <= st2 " << endl;
    else cout << "st1 is not less than or equal to st2 " << endl;

    if (st1 > st2)
        cout << "st1 > st2 " << endl;
    else cout << "not (st1 >  st2) " << endl;

    if (st1 >= st2)
        cout << "st1 >= st2 " << endl;
    else cout << "not (st1 >=  st2) " << endl;

    cout << "operator >> " << endl;

    //Open the data file
    ifstream input("A9_input.txt");
    if (input.fail()) {
        cout << "unable to open input file A9_input.txt, Exiting..... ";
        system("pause");
        return 0;
    }
    MyString temp1;
    MyString temp2("aaa");
    input >> temp1;
    input >> temp2;
    cout << "first element of input file: " << temp1 << endl;
    cout << "second element of input file: " << temp2 << endl;
    input.close();

    cout << "MyString says farewell....." << endl;
    system("pause");
    return 0;
}
Felix Chang
  • 9
  • 2
  • 2
  • 5
  • So how far do you get before the error appears? – Bo Persson Jul 09 '16 at 05:30
  • You're trying to do something with the string pointed to by `value`, but `value` is `NULL`. – Barmar Jul 09 '16 at 05:34
  • 1
    You've tried to walk before you can run - this code is a mess. No null checks, `operator+` has numerous bugs in it - it shouldn't even compile since you use a `const MyString` as the first argument to strcat, never mind that `strcat` requires that the destination buffer be large enough to receive the second additional text. In `operator+=` you use `newMemory` to store the old address... Your `operator>>` is weird, your comparison operators compare pointers, and your destructor assumes you own the pointer which is not the case if constructed with `MyString(char* s)`. – kfsone Jul 09 '16 at 06:24

2 Answers2

1

Your copy constructor never sets the target's value, so when you try to use the new string, value is uninitialized. Instead of using the local variable newCopy, you should assign to value.

MyString::MyString(const MyString& newWord) {

    //perform a deep copy to copy each of the value to a new memory
    size = newWord.size;
    value = new char[size];

    for (int ii = 0; ii < size; ii++) {
        value[ii] = newWord.value[ii];
    }
}

Also, the constructor that takes char* chars must make a copy of chars. Otherwise, the pointer could become invalid if the parameter was a local array that has gone out of scope, or a dynamic array that was deleted. Also, since the destructor does delete[] value;, it requires that this be dynamically-allocated, which wouldn't be correct when you initialize it from string literals.

//constructor with an argument
MyString::MyString(char* chars) {

    size = strlen(chars);
    value = new char[size];
    for (int i = 0; i < size; i++) {
        value[i] = chars[i];
    }
}
Barmar
  • 596,455
  • 48
  • 393
  • 495
  • I still get this error when I tried to implement the code you suggested: Exception thrown at 0x0F5E0E09 (ucrtbased.dll) in ConsoleApplication5.exe: 0xC0000005: Access violation writing location 0x005D2000. Do you have any idea with this one? Sorry, I've just been using C++ for two months. Thanks for your great help! – Felix Chang Jul 09 '16 at 20:36
  • 1
    Sorry, you have lots of code there, I can't see all the possible problems you have with it. You need to learn how to use a debugger so you can look at the variables at each step of the program, and see when they become invalid. – Barmar Jul 09 '16 at 23:52
0

There is a whole series of issues to address:

First, you allow value to be 0/NULL/nullptr (best use the new c++11 nullptr keyword!). If you do so, you need to consider this possibility in every function and operator, else you will try to access an invalid memory location (e. g. when doing strlen(value)). Exactly this is what the exception means (0 is no valid memory address in most cases, some exceptions exist for embedded systems - where you might, however, break the entire system if writing to it!): you ran into such a situation.

To avoid this hassle in every function/operator, you might prefer to always set value to an existing empty string aready in the constructor, just as std::string does (exception: not every implementation of std::string does accept std::string(nullptr)). You avoid another problem for the user, too: you enabled him to get two different string representations for string of size 0: nullptr and "".

Another issue is that you are not copying the string in case of the constructor acceptying char*, but deleting it in the destructor. Problem: the argument could be a char buffer allocated on the stack:

char buffer[64];
/* ... */
return MyString(buffer);

It might be efficient to take owner ship of an existing string in many cases (something impossible with std::string – safe in every situations, inefficient in some), but you can get deep problems in others (as above, or if assigning a string literal return MyString("hello world"); (OK, this is problematic anyway, as literals are char const* in C++ - but many compilers let you get away with it, just raising a warning). If you wanted to allow taking ownership, I would not do it by default (copy the string), and add another constructor explicitely allowing it (MyString(char*, bool takeOwnership = false), together with (MyString(char const*) always copying - note the const). Of course, you need to remember somewhere, if you have ownership. Only delete the internal representation, if you have ownership (the default...).

You are comparing the char* values via ==, !=, <, >, ... operators. Be aware that you get equality only if both value members point to exactly the same string, but not, if two different strings are literally equal:

char b1[] = {'a', 'b', 'c', 0 };
char b2[] = {'a', 'b', 'c', 0 };
bool isEqual = MyString(b1) == MyString(b2);

isEqual will be false! I assume you did not intend this, rather the same behaviour as std::string provides. An even more serious problem are the operators <, <=, >, >=. It is legal to compare with these only if both pointers point to the same array or one past the end (b3 = b1 + 2), otherwise, it is undefined behaviour.

To get the behaviour I assume you want to have, you need to use strcmp:

bool operator X(MyString const& l, MyString const& r)
{
    return strcmp(l, r) X 0;
}

Where you replace X with the appropriate operator (note that you can define != as !(l == r), <= as !(l > r), etc.).

And please, please, please: Don't do such stuff as:

if(a == b)
    return true;
else
    return false;

or

bool isWhatEver;
if(c != d)
    isWhatEver = true;
else
    isWhatEver = false;

Simply write

return a == b;

isWhatEver = c != d;

Addendum:

You are violating the rule of three, you did not provide an assignment operator: operator=(MyString const& other). The rule of three actually became the rule of five with C++11; you might want to add a move constructor and a move assignment operator, too. Before implementing, have a look at here, that might be very useful for you...


Edit:

It is almost impossible to tell just from the address, what went wrong. You did not provide any stack trace and not fixed code of yours...

So I allowed myself to fix your class to what I think you might have intended. This version ran through all of your tests, at least, content of the input file was simply 'hello world'. Left out move constructor (hint: use std::swap for this). I still do not guarantee it to be bugfree... Some advise: don't just take my code, have a close look at what I did differently to see what might have gone wrong. One last thing: I made operator>> safe against failure because of exceeding buffer size using an std::vector.

class MyString
{
public:

    //default constructor
    MyString();

    MyString(char const* chars);

    //copy constructor
    MyString(const MyString &);

    int length() const;

    //destructor
    ~MyString();

    //operator overloads
    char& operator[](int index);
    friend MyString operator+(const MyString& newWord, const MyString& newWord2);
    MyString& operator+=(const MyString& newWord);
    MyString& operator=(const MyString& newWord)
    {
        char* newValue = new char[newWord.size];
        delete value;
        value = newValue;
        size = newWord.size;
        memcpy(value, newWord.value, size);
        return *this;
    }
    MyString& operator=(char const* chars)
    {
        size_t newSize = strlen(chars);
        char* newValue = new char[newSize];
        delete value;
        value = newValue;
        size = newSize;
        memcpy(value, chars, size);
        return *this;
    }
    friend ostream& operator<<(ostream& newWord, const MyString& newWord2);
    friend istream& operator >>(istream& newWord, MyString& newWord2);
    friend bool operator==(const MyString& newWord, const MyString& newWord2);
    friend bool operator!=(const MyString& newWord, const MyString& newWord2);
    friend bool operator<(const MyString& newWord, const MyString& newWord2);
    friend bool operator<=(const MyString& newWord, const MyString& newWord2);
    friend bool operator>(const MyString& newWord, const MyString& newWord2);
    friend bool operator>=(const MyString& newWord, const MyString& newWord2);

private:
    char* value;
    int size;
};

//default constructor
MyString::MyString()
{
    value = new char[1];
    *value = 0;
    size = 0;
}

//copy constructor
MyString::MyString(const MyString& newWord)
{
    //perform a deep copy to copy each of the value to a new memory
    size = newWord.size;
    value = new char[size];
    memcpy(value, newWord.value, size);
}

//constructor with an argument
MyString::MyString(char const* chars)
{
    if(chars)
    {
        size = strlen(chars);
        value = new char[size];
        memcpy(value, chars, size);
    }
    else
    {
        value = new char[1];
        *value = 0;
        size = 0;
    }
}

//find length
int MyString::length() const
{
    return size;
}

//find the value of each index
char& MyString::operator[](int index)
{
    return value[index];
}

//operator + (concatenate)
MyString operator+(const MyString& newWord, const MyString& newWord2)
{
    MyString concatenated;
    concatenated.value = new char[newWord.size + newWord2.size + 1];
    memcpy(concatenated.value, newWord.value, newWord.size);
    memcpy(concatenated.value + newWord.size, newWord2.value, newWord2.size + 1);
    return concatenated;
}

//operator += (append)
MyString& MyString::operator+=(const MyString& newWord)
{
    if(newWord.size > 0)
    {
        char * newMemory = value;
        value = new char[size + newWord.size + 1];
        memcpy(value, newMemory, size);
        memcpy(value + size, newWord.value, newWord.size);
        delete[] newMemory;
        size += newWord.size;
    }
    return *this;
}

//ostream operator
ostream& operator<<(ostream& newWord, const MyString& newWord2)
{

    newWord << newWord2.value;
    return newWord;
}

//istream operator
istream& operator >>(istream& newWord, MyString& newWord2)
{
    std::vector<char> v;
    for(;;)
    {
        char c = newWord.get();
        if(newWord.eof() || isspace(c))
        {
            break;
        }
        v.push_back(c);
    }
    if(!v.empty())
    {
        newWord2 = v.data();
    }
    return newWord;
}

//all boolean operators
bool operator==(const MyString& newWord, const MyString& newWord2)
{
    return strcmp(newWord.value, newWord2.value) == 0;
}

bool operator!=(const MyString& newWord, const MyString& newWord2)
{
    return !(newWord == newWord2);
}

bool operator<(const MyString& newWord, const MyString& newWord2)
{
    return strcmp(newWord.value, newWord2.value) < 0;
}
bool operator>(const MyString& newWord, const MyString& newWord2)
{
    return strcmp(newWord.value, newWord2.value) < 0;
}

bool operator<=(const MyString& newWord, const MyString& newWord2)
{
    return !(newWord > newWord2);
}

bool operator>=(const MyString& newWord, const MyString& newWord2)
{
    return !(newWord < newWord2);
}

//destructor to release memory
MyString::~MyString()
{
    delete[] value;
}
Community
  • 1
  • 1
Aconcagua
  • 19,952
  • 4
  • 31
  • 51
  • still get this error when I tried to implement the code you suggested: Exception thrown at 0x0F5E0E09 (ucrtbased.dll) in ConsoleApplication5.exe: 0xC0000005: Access violation writing location 0x005D2000. Do you have any idea with this one? Sorry, I've just been using C++ for two months. Thanks for your great help! – Felix Chang Jul 09 '16 at 20:38
  • This is too little information to guess what the actual error might be. Added some code that might be of use for you. – Aconcagua Jul 10 '16 at 07:56