String class + operator

Question

I am having this problem where I am trying to overload the add function for my custom string class, so I make a temporary object to add both of the objects lengths and arrays together, however I keep getting segmentation faults and I don't know why. I have tested both my assignment operator works and my equality operator and they are both perfectly working.

myString myString::operator+(const myString& str) const{
int p = 0;
myString tmp;
tmp.sLength = sLength + str.sLength;
tmp.s = new char[tmp.sLength];
while (p != (sLength - 1))
{
    tmp.s[p] =  s[p];

    p++;
}

while (p != (tmp.sLength - 1))
{
    tmp.s[p] = str.s[(p - sLength)];

    p++;    
}


return tmp;
//tmp.s = NULL;
  }
   myString& myString::operator=(const myString& str)
 {
  if (this != &str)
  {
    if ( s != NULL)
    {
        if (str.s == NULL)
        {
            sLength = 0;
            s = NULL;
        }
        else
        {

                delete [] s;

            s = new char [str.sLength];
            sLength = str.sLength;
            for (int i = 0; i < sLength; i++)
            s[i] = str.s[i];
        }
    }
}
return *this;   
}

Please post a [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve). Is your code compliant with [The Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three)? — MikeCAT, Nov 19 '16 at 02:59
You need to try gdb: in gdb run the code ('run'), when it faults, climb the stack to code you recognize (using 'up'). Look around this code. Try "p *this", see the state of your object, or "p localVar" — 2785528, Nov 19 '16 at 03:02

score 1 · Answer 1 · answered Nov 19 '16 at 03:20

When this loop

while (p != (sLength - 1))
{
    tmp.s[p] =  s[p];

    p++;
}

ends its iterations variable p will be equal to sLength -1.

Thus in this loop

while (p != (tmp.sLength - 1))
{
    tmp.s[p] = str.s[(p - sLength)];

    p++;    
}

in the first iteration you have

    tmp.s[sLength -1] = str.s[(sLength -1 - sLength)];

that is

    tmp.s[sLength -1] = str.s[(-1)];
                              ^^^^

Also it is not clear why the loops use conditions like that p != sLength - 1. Why do not they use the conditions like p != sLength ?

The copy assignment operator is also wrong.

For example, if s != NULL you just assign to it NULL without deleting the previously allocated memory.

if ( s != NULL)
{
    if (str.s == NULL)
    {
        sLength = 0;
        s = NULL;
        ^^^^^^^^

And moreover if s is equal to NULL then you assign to it nothing though str.s can be a non null pointer.

Also a question arises why do you use here the following condition in the loop

        for (int i = 0; i < sLength; i++)
                        ^^^^^^^^^^^
        s[i] = str.s[i];

instead of

        for (int i = 0; i < sLength - 1; i++)
                        ^^^^^^^^^^^^^^^
        s[i] = str.s[i];

as it is in the operator +?

String class + operator

1 Answers1