1

My college teacher assigned a homework about simulating string, and here's one selection of my code:

// MyString.h
#include <iostream>
#include <cstdio>
#include <string.h>

using std::ostream;
using std::istream;

class MyString{
protected:
    char *_data;                // data index
    int  _len;                  // index size
public:
/////////////////////////////////////////////////////////
    char* data()const{
        return _data;
    }
/////////////////////////////////////////////////////////

    MyString& operator=(const char* &rhs){
        _data = const_cast<char*>(rhs);
        _len  = strlen(_data)
    }

    MyString& operator=(const MyString& rhs){
        (*this) = rhs._data;
    }
/////////////////////////////////////////////////////////
    friend ostream &operator<<( ostream&, const MyString & );
    friend istream &operator>>( istream&, MyString & );
};


ostream& operator <<(ostream& os, const MyString& str){
    os << str.data();
    return os;
}

istream& operator >>(istream& is, MyString& str){
    char buffer[0xff];
    is.getline(buffer, 0xff);
    str = buffer;
    return is;
}

I totally have no idea why operator = causes infinite loop here...please help...thanks in advance

Edit: There goes my buggy code. I have edited where cause the causes infinite loop issue but the rest of them maybe still bugged, I'll try to fix them soon, thank you all!

MyString.h

#include <iostream>
#include <cstdio>
#include <string.h>

using std::ostream;
using std::istream;

class MyString{
protected:
    char *_data;                // data index
    int  _len;                  // index length
    static int _total_MyString; // number of instance created
public:
/////////////////////////////////////////////////////////
    // basic initialization
    MyString(){
        _data = NULL;
        _len = 0;
        _total_MyString++;
    }

    // init with defined string
    MyString(const char *s):MyString(){
        (*this)= s;
    }

    // init with same class data
    MyString(const MyString & s):MyString(s._data){}

    // finalizer
    ~MyString(){
        _total_MyString--;
        delete _data;
    }

/////////////////////////////////////////////////////////
    static int total_MyString(){
        return _total_MyString;
    }
/////////////////////////////////////////////////////////
    char* data()const{
        return _data;
    }
/////////////////////////////////////////////////////////

    MyString& operator=(const char* &rhs){
        char* temp = const_cast<char*>(rhs);
        _len  = strlen(temp);
        _data = new char [_len + 1];
        strcpy(_data, rhs);
        return (*this);
    }

    MyString& operator=(const MyString& rhs){
        const char* temp = rhs._data;
        return (*this = temp);
    }

/////////////////////////////////////////////////////////
    MyString& operator+=(const char* &rhs){
        _len += strlen(rhs);
        strcat(_data, rhs);
        return *this;
    }

    MyString& operator+=(const MyString& rhs){
        const char* temp = rhs._data;
        (*this) += temp;
        return *this;
    }

/////////////////////////////////////////////////////////
    inline char & operator[](const int pos){
        return _data[pos];
    }

/////////////////////////////////////////////////////////
    unsigned length() const{
        return _len;
    }

/////////////////////////////////////////////////////////
    friend ostream &operator<<( ostream&, const MyString & );
    friend istream &operator>>( istream&, MyString & );
/////////////////////////////////////////////////////////
    bool operator==(const MyString& str){
        return !strcmp(_data, str._data);
    }

    friend bool operator!=(const MyString& str, const MyString& str2){
        return strcmp(str._data, str2._data) != 0;
    }
};

ostream& operator <<(ostream& os, const MyString& str){
    os << str.data();
    return os;
}

istream& operator >>(istream& is, MyString& str){
    char buffer[0xff];
    is.getline(buffer, 0xff);
    const char* temp = &buffer[0];
    str = temp;
    return is;
}

MyString.cpp

#include "MyString.h"

using namespace std ;

int MyString::_total_MyString = 0;

int  main()
{

    MyString Dstr1("String"), Dstr2("Test String 2"), Dstr3(Dstr1);

    cout << "Dstr1 is: " << Dstr1 << endl;
    cout << "Dstr2 is: " << Dstr2 << endl;
    cout << "Dstr3 is: " << Dstr3 << endl;
    cout << "Total MyString_Derived is: " << MyString_Derived::total_MyString() << endl;

/////////////////////////////////////////////////////////

    cout << "Give one word "<< endl;
    cin >> Dstr1;
    cout << "Dstr1 is: " << Dstr1 << endl;

/////////////////////////////////////////////////////////

    Dstr2 = Dstr1;
    cout << "Dstr2 is: " << Dstr2 << endl;
    return 0;
/////////////////////////////////////////////////////////

    Dstr3 += Dstr1;
    cout << "Dstr3 is: " << Dstr3 << endl;

/////////////////////////////////////////////////////////

    cout << "Dstr3 is: ";
    for(int i = 0; i<Dstr3.length(); i++)
        cout << Dstr3[i];
    cout << endl;

    for(int i = 0; i<Dstr3.length(); i++)
        Dstr3[i] = 'a' + i;
    cout << "Dstr3 is: " << Dstr3 << endl;

/////////////////////////////////////////////////////////

    cout << "Dstr1 is: " << Dstr1 << endl;
    cout << "Dstr2 is: " << Dstr2 << endl;
    cout << "Dstr3 is: " << Dstr3 << endl;
    cout << "Compare Dstr1 == Dstr2 is: " << (Dstr1 == Dstr2) << endl;
    cout << "Compare Dstr1 == Dstr3 is: " << (Dstr1 == Dstr3) << endl;
    cout << "Compare Dstr1 != Dstr2 is: " << (Dstr1 != Dstr2) << endl;
    cout << "Compare Dstr1 != Dstr3 is: " << (Dstr1 != Dstr3) << endl;
}
Compeador
  • 27
  • 7
  • This is likely not the reason for this issue, but your `operator =` claim to return references, but they don't. This is UB. – Jodocus May 30 '18 at 06:14
  • Maybe due to self-assignment? see [here](https://stackoverflow.com/questions/5608556/assignment-operator-self-assignment). – tangoal May 30 '18 at 06:14
  • Your code does not compile either, it should have to read `MyString& operator=(const char* const &rhs)`. Whether this is a good idea like that is another topic. – Jodocus May 30 '18 at 06:17
  • This is not enough code to reproduce the issue. What do the class constructor's look like? Also, `operator=` needs to make a *copy* of the source data, but it is not doing so, so there is undefined behavior in `operaror>>` caused by a bad `operator=` implementation. – Remy Lebeau May 30 '18 at 06:28
  • Your assignment operator is storing the address of an object with automatic duration. Dereferencing that pointer after `>>` has returned is undefined. (This exercise is mainly about dynamic memory allocation and pointer experience. Review your book and notes.) – molbdnilo May 30 '18 at 06:56
  • After the edit, there are still errors in this code. You are NOT managing the `_data` pointer correctly in some places. Notably, `operator=` and `operator+=` are still broken. And the operators and even the constructors are not implemented very efficiently, either, mainly because you are completely ignoring the `_len` value, you are not utilizing it during copy operations. – Remy Lebeau May 30 '18 at 15:27

1 Answers1

0

Consider the operator>>

istream& operator >>(istream& is, MyString& str){
    char buffer[0xff];
    is.getline(buffer, 0xff);
    const char* temp = &buffer[0];
    str = temp;
    return is;
}

buffer is a local variable. you create a pointer to this local array and assing it to str. That calls

MyString& operator=(const char* &rhs){
    _data = const_cast<char*>(rhs);
    _len  = strlen(_data);
    return (*this);
}

Which is interesting: as a parameter you give the reference to a pointer, which is const, and you then strip the const-ness from it and assign it to _data. Now char* _data is pointing to the local array buffer. What happens at the end of the function, when the local array is destroyed? The pointer becomes invalid.

You need to implement a deep copy.

JHBonarius
  • 10,824
  • 3
  • 22
  • 41