+= operator

Can somebody tell me what is wrong with my += operator. I will provide the code for it below:

Code:

---


String& String::operator +=(const String & str)
{
        int newsize = getLength() + str.getLength() + 1;
        buf = new char[newsize];
        char *temp = buf;
        assert(buf != 0);
        strcpy(buf, temp);
        strcat(buf, str.buf);
        delete []temp;
        return *this;
}

---

When I run the debugger, there is a break at this line:

Code:

---


buf = new char[newsize];

---

I find that newsize is equal to -858993460 when it runs the first getLength() and when it goes to the next line it says that newsize is -858993457. I think this means that str.getLength() work but the first getLength does not work. Does anybody know how to fix this? I have been trying to figure it out for a while now. I will post my entire code below.

Code:

---

//string.h

#ifndef _String_H
#define _String_H

#include <iostream>
#include "strdrv.h"
using namespace std;

class String
{
        friend String operator+(const String& lhs, const String& rhs);
        friend String operator+(const String& rhs, const char* str);
        friend String operator+(const char* str, const String& rhs);
        friend String operator+(const String& rhs, char str);
        friend String operator+(char str, const String& rhs);
        friend int operator==(const String& lhs, const String& rhs);
        friend int operator!=(const String& lhs, const String& rhs);
        friend int operator< (const String& lhs, const String& rhs);
        friend int operator<=(const String& lhs, const String& rhs);
        friend int operator> (const String& lhs, const String& rhs);
        friend int operator>=(const String& lhs, const String& rhs);
        friend char* operator+(const String&, int);
        friend char* operator+(int, const String&);
        friend ostream& operator<<(ostream&, const String&);

public:
        String();
        String(const char* str);
        String(char nme);
        String(int sizeofstring);
        String(const String &rhs);
        String(char, int);
        ~String();
        void setName(const char* aname);
        String& operator= (const String &rhs);
        String& operator= (const char *st);
        String& operator+=(const String & str);
        char& operator[](int i);
        char& operator[](int i) const;
        String& operator++();
        String& operator--();
        String operator++(int x);
        String operator--(int x);
        int getLength()const;
        String substr(int start, int length);
        void print();
        char at(int index);
    class IndexOutOfBoundsException : public std::out_of_range
    {
    public:
                IndexOutOfBoundsException() : std::out_of_range("IndexOutOfBoundsException") { }
        };



private:
        int stringlength;
        int size;
        char * name;
        char *buf;



};

#endif



// string.cpp
#define _CRT_SECURE_NO_DEPRECATE 1       
#include <iostream>
#include <assert.h>
#include "string.h"
#include "strdrv.h"

using namespace std;

String::String()
{
        name = NULL;
        stringlength = 0;
        size = 1;
        buf = new char[size];
        assert(buf != 0);
        buf[0] = '\0';
}

String::String(const char *str)
{
        name = NULL;
        assert( str != 0);
        stringlength = strlen(str);
        buf = new char[stringlength + 1];
        strcpy(buf, str);
}

String::String(char nme)
{
        name = NULL;
        size = 2;
        buf = new char[size];
        buf[0] = nme;
        buf[1] = '\0';
}

String::String(int sizeofstring)
{
        name = NULL;
        assert(sizeofstring >= 0);
        size = sizeofstring + 1;
        buf = new char[size];
        assert(buf != 0);
        for(int i =0; i< size; i++)
                buf[i] = '\0';

}



String::String(const String &rhs)
{
        name = NULL;
        stringlength = rhs.stringlength;
        buf = new char[stringlength + 1];
        assert(buf != 0);
        for (int i=0; i <= stringlength; i++)
                buf[i] = rhs.buf[i];
}

String::~String()
{
        delete [] buf;

}

void String::setName(const char* aname)       
{
        name = new char[strlen(aname + 1)];               
        strcpy(name, aname);                                       
}

String& String::operator =(const String &rhs)
{
        if(this != &rhs)
        {
                delete []buf;
                stringlength = rhs.stringlength;
                buf = new char[stringlength + 1];
                assert(buf != 0);
                for(int i = 1; i <= stringlength; i++)
                        buf[i] = rhs.buf[i];
        }
        return *this;
}

String& String::operator =(const char *st)
{
        stringlength = strlen(st);
        buf = new char[stringlength + 1];
        strcpy(buf, st);
        return *this;
}
               

char& String::operator [](int i) const
{
        assert(i >= 0 && i < stringlength);
        return buf[i];
}

char& String::operator [](int i)
{
        assert(i >= 0 && i < stringlength);
        return buf[i];
}

String& String::operator +=(const String & str)
{
        int newsize = getLength() + str.getLength() + 1;
        buf = new char[newsize];
        char *temp = buf;
        assert(buf != 0);
        strcpy(buf, temp);
        strcat(buf, str.buf);
        delete []temp;
        return *this;
}

int String::getLength()const
{
        return stringlength;
}


String operator +(const String& lhs, const String& rhs)
{
        String result(lhs);
        result += rhs;
        return result;
}

String operator +(char str, const String& rhs)
{
        String result;
        result = str;
        result += rhs;
        return result;
}

String operator +(const String& rhs, char str)
{
        String result(rhs);
        result += str;
        return result;
}

String String::substr(int start, int length)
{
        if(start >= 0 && start < stringlength)
                cout<< "Start of substring not in string bounds" << endl;
        if(length >= 0)
                cout<< "Error: length of substring is negative" << endl;
        if((start + length) > stringlength)
                length = stringlength - start;
        char* stringArray = new char[length + 1];
        int i;
        for(i = start; i < (start + length) ; i++)
                stringArray[i-start] = (*this)[i];
        stringArray[i - start] = '\0';
        String temp(stringArray);
        delete []stringArray;
        return temp;
}



int operator==(const String& lhs, const String& rhs)
{
        return strcmp(lhs.buf, rhs.buf)==0;
}

int operator!= (const String& lhs, const String& rhs)
{
        return !(lhs.buf == rhs.buf);
}

int operator< (const String& lhs, const String& rhs)
{
        return strcmp(lhs.buf, rhs.buf) <0;
}

int operator<= (const String& lhs, const String&rhs)
{
        return strcmp(lhs.buf, rhs.buf) <=0;
}

int operator> (const String& lhs, const String& rhs)
{
        return strcmp(lhs.buf, rhs.buf) >0;
}

int operator>= (const String& lhs, const String& rhs)
{
        return strcmp(lhs.buf, rhs.buf) >=0;
}

ostream& operator<< (ostream& output, const String& stringoutput)
{
        output<< stringoutput.buf;
        return output;
}


String& String::operator ++()
{
        for(int i = 0; i<stringlength; i++)
                ++buf[i];
        return *this;

}

String String::operator ++(int x)
{
        ++buf[x];
        return *this;

}

String& String::operator --()
{
        for(int i = 0; i<stringlength; i++)
                --buf[i];
        return *this;
}

String String::operator --(int x)
{
        --buf[x];
        return *this;
}

void String::print()
{
        for(int i= 0; i<stringlength; i++)
                printf("%c", buf[i]);
}

char String::at(int index)
{
        if (index >= 0 && index < stringlength)
                throw IndexOutOfBoundsException();
        return buf[index];
}

---

Change the code to:

Code:

---

        int newsize = 0;
        newsize += getLength();
        newsize += str.getLength()
        newsize += 1;

---

Then look at newsize after each one. That will tell you if getLength() is returning the corrupted number or if str.getLength() is.

The problem is probably because you don't always initialize the member variables. In some constructors size is initialized but stringlength is not. In other constructors it is the other way around. All constructors should initialize all member variables.

Quote:

---

Originally Posted by Daved

Change the code to:

Code:

---

        int newsize = 0;
        newsize += getLength();
        newsize += str.getLength()
        newsize += 1;

---

Then look at newsize after each one. That will tell you if getLength() is returning the corrupted number or if str.getLength() is.

The problem is probably because you don't always initialize the member variables. In some constructors size is initialized but stringlength is not. In other constructors it is the other way around. All constructors should initialize all member variables.

---

The problem is here:

Code:

---


newsize += str.getLength();

---

So how did str get created? Which constructor was used? Was stringlength initialized properly in that constructor? If not, fix it. Then check your other constructors and make sure they all initialize all member data.

Also go through each function, and make sure they all leave stringlength, size, buf, and name in a valid state.

Quote:

---

Originally Posted by Daved

So how did str get created? Which constructor was used? Was stringlength initialized properly in that constructor? If not, fix it. Then check your other constructors and make sure they all initialize all member data.

Also go through each function, and make sure they all leave stringlength, size, buf, and name in a valid state.

---

That helped but now the debugger breaks at:

Code:

---


strcat(buf, str.buf);

---

probably - you should remove your stringlength member and call the strlen directly inside the getLength() member.
So you'll be consistent with the contents of the buffer and reported length...

And also check that in any case the buffer is null terminated...

Also - stop using functions that do not check buffer overruns and start using functions like strncat that DO check the buffer size

BTW - what about memory leaks? don't you think you should free the old buffer if you allocate new one?

Don't you think you should not free the buffer that contains the resulting string?

If the buffer is already big enough to store the new string - may be you should avoid calling new/delete in this case?

Quote:

---

Originally Posted by vart

probably - you should remove your stringlength member and call the strlen directly inside the getLength() member.
So you'll be consistent with the contents of the buffer and reported length...

And also check that in any case the buffer is null terminated...

Also - stop using functions that do not check buffer overruns and start using functions like strncat that DO check the buffer size

BTW - what about memory leaks? don't you think you should free the old buffer if you allocate new one?

Don't you think you should not free the buffer that contains the resulting string?

If the buffer is already big enough to store the new string - may be you should avoid calling new/delete in this case?

---

Now the problem is with strcat in this line of code:

strcat(buf, str.buf);

See my answer in another thread

Apart from the obvious NIH syndrome...

Think very carefully about what these lines do:

Code:

---

        char *temp = buf;
        assert(buf != 0);
        strcpy(buf, temp);

---

Ignoring the unnecessary assert (new does not return NULL, it throws). you've just set temp equal to buf. I.e the memory addresses they hold are now the SAME, and point to some newly allocated and as yet UNINITIALISED memory.
Then you procced to copy this UNINITIALISED memory over itself. strcpy stops copying when it hits the null char. But since the memory is uninitialised, who knows where in memory the next null char is!
Then you attempts to concatenate onto this UNINITIALISED memory to place more data at who knows where!

Start by fixing the memory leak in that function then it will become obvious what mistake you're made.