bug in my operator += overloading method?

Hi all,
I've got an application that SIGSEGV a lot, and it always does so when I'm doing += with a string object. Now I've run "top" while my programming is running, and I don't see "free mem" run to zero, however the SIGSEGV is due to a malloc_consolidate leading back to my string's += method. :(
There could certainly be a massive memory leak somewhere else in my program, but since it always shows up with += I've been trying to see if it's my string class that's causing the problem. Also, wouldn't "top" show "free mem" of 0 if it was a problem with memory?

The malloc_consolidate always points to the "new" line below.

Can anyone see any error in this code? It's all inline -- that shouldn't make a difference should it??

Code:

---


class JString
{
  public:
.....................
      ~JString()
      {
        delete[] myCstring;
        myCstring = 0;
        myLength = 0;
      }; //end destructor
..............
      //returns number of chars
      int length() const
      {
        return strlen(myCstring);
      };  // end length()

      ///////////////////////////////////////////////////////
      // OPERATOR OVERLOADING
      ///////////////////////////////////////////////////////
      // postcondition: concatenates x onto object
      // assertion: fails if memory can't be allocated
      JString& operator +=(const JString & x)
      {
        int lastLocation = length();          // location of '\0'
        int newLength = lastLocation + x.length(); // object + added JString

        char * copyStr = x.myCstring;          // copy this one
        char * delStr  = myCstring;            // delete this one
        bool doDelete = false;                // delete old JString?

        // check to see if local buffer not big enough
        if (newLength >= myLength)
        {
            char * newBuffer;
            newBuffer = new char[newLength + 1];  //<<<<<<<<<<-------- SIGSEGV here
            strcpy(newBuffer, myCstring); // copy into new buffer
            doDelete = true;
            myLength = newLength + 1;            // update information
            myCstring = newBuffer;
        } //end if

        // now catenate x to end of myCstring
        strcpy(myCstring + lastLocation, copyStr);

        if (doDelete)          // delete here for aliasing
        {
            delete [] delStr;    // old JString
        } //end if

        return *this;

      }; //end +=
......................


      //maximum length of JString read from input
      enum { maxLength = 1024 };



  private:

    char *  myCstring;  // private C-style JString
    int      myLength;  // length of ourselves


}; //end class JString

---

I only find one mistake

Code:

---

      JString& operator +=(const JString & x)
      {
        int lastLocation = length();          // location of '\0'
        int newLength = lastLocation + x.length(); // object + added JString

        char * copyStr = x.myCstring;          // wrong!myCstring is a private data member,you can't access it directly
        char * delStr  = myCstring;            // delete this one
        bool doDelete = false;                // delete old JString?

        // check to see if local buffer not big enough
        if (newLength >= myLength)
        {
            char * newBuffer;
            newBuffer = new char[newLength + 1];  //<<<<<<<<<<-------- SIGSEGV here
            strcpy(newBuffer, myCstring); // copy into new buffer
            doDelete = true;
            myLength = newLength + 1;            // update information
            myCstring = newBuffer;
        } //end if

        // now catenate x to end of myCstring
        strcpy(myCstring + lastLocation, copyStr);

        if (doDelete)          // delete here for aliasing
        {
            delete [] delStr;    // old JString
        } //end if

        return *this;

      }; //end +=

---

Quote:

---

Originally posted by jinhao
I only find one mistake

Code:

---

        char * copyStr = x.myCstring;          // wrong!myCstring is a private data member,you can't access it directly

---

---

There is nothing wrong with that - you can access private members from the same class.

The fact is, the problem probably lies elsewhere in the code. Segmentation faults in malloc_consolidate() usually occur when you're trying to allocate some memory after you've corrupted the heap. Check elsewhere in the code to make sure you're not double deleting objects or overwriting your array bounds.

Thanks for the great feedback. I'll go back and review all my code -- I was concentrating only on where the crash occured. I'll let you know what I find. :)

Just to resolve the thread in case anyone reads it in the future, I did find the problem, and it was an array index violation.

I was doing something like
char *data = new char [size];

then much later

data[size] = '\0';

oops.


I found this by using MALLOC_CHECK_, which is a very cool thing. If you call your program 'myProgram' then add this env variable value in front of it, so you call it like this:

prompt> MALLOC_CHECK_=1 ./myProgram

then, it will tell you:
free(): invalid pointer 0x78976
etc., as the program executes, which is a very handy thing. :) Now, using gdb I haven't figured out how to find what line actually causes the problem, but by inserting strategic exit(0) calls into your code, you can run it and see if it exits before the "free(): invalid pointer" msg.