Memory Management : arrays and dynamic allocation with new operator

Question

Q. In terms of Memory Management, what error does occur?

class String
{
public:
 String(const char right[]);
 String& operator= (const String& right);
 int length() const;
private:
 char* buffer;
 int len;
}

int String::length() const {return len;}

String::String(const char right[])
{
 len = 0;
 while (right[len] != '\0')
   len++;
 buffer = new char[len+1];
 for (int i = 0; i < len; i++)
   buffer[i] = right[i];
 buffer[len] = '\0';
}

String& String::operator= (const String& right)
{
 if (this != &right)
  { 
    delete[] buffer;
    len = right.length();
    char* buffer = new char[len + 1];
    for (int i = 0; i < len; i++)
      buffer[i] = right[i];
    buffer[len] = '\0';
  }
  return *this;
}

Answer. I have no clue... Could you help me? This one seems ok too. new and it is also deleted. Where is the memory leak?

Please let me know. Thanks,

In addition to all the answers, you should always remember one thing: if you use operator `new`, you should use operator `delete` somewhere. — SpongeBobFan, Apr 23 '13 at 08:34

score 1 · Answer 1 · edited May 23 '17 at 12:28

You need to provide a constructor which allocates the pointer member(infact all constructors of your class should do that) and destructor which deallocates it.

Also, You need to provide a copy constructor which performs a deep copy of the pointer member.

String::String(const String& obj)
{
   ....
}

Good Read:
What is The Rule of Three?

Also, String is an awful name for a class especially since there exists a std::string.

score 1 · Answer 2 · answered Apr 23 '13 at 04:38

Rule of three: if a class defines a destructor or a copy constructor or a copy assignment operator, it probably has to define all three.

Your code vioaltes this rule by not providing a destructor and a copy constructor while providing a copy assignment operator.

score 0 · Answer 3 · answered Apr 23 '13 at 06:23

When your String objects are destroyed, the default constructor is called since you did not define one. The default destructor will not delete[] your char array for you, you must declare and define a destructor to do this.

Also, you will find that using strcpy()/strncpy() is much faster than a simple loop to copy each char. (at least when compiling with GCC).

Peter R · Accepted Answer · 2013-04-23T09:03:37.943

There are 4 issues with your code.

Your assignment operator has the following line: char* buffer = new char[len + 1]; This is declaring a new pointer variable and not using the class member. This will allocate memory to a pointer that then goes out of scope and can never be deleted and will be a memory leak and make your class not function.
The lack of a destructor means that your class leaks memory. Your destructor is responsible for freeing the memory that your are allocating with new[].
The lack of a copy constructor means that your String class cannot be used in many situations including in an STL container. The STL containers require that types have a public copy constructor, assignment operator and desctructor.
You are not enforcing your class invariants in the face of exceptions. The goal is to make your class exception safe. In the changes I made I tried to achieve the strong exception guarantee and conserve the class state in the face of exceptions. Considering exception safety is an important aspect of creating your own classes - especially library classes such as String.

Here's the code that attempts fixes all of those issues. To keep the code short I am putting the implementation into the class rather than separating it. Of course, in real code you should use std::string.

class String
{
public:
    String() : len(0), buffer(nullptr)
    {}

    String(const char right[]) : len(0), buffer(nullptr)
    {
        if(!right)
            return;

        int temp_len = strlen(right);
        buffer = new char[temp_len+1];
        len = temp_len; // only set the length after the allocation succeeds
        strcpy(buffer, right);
    }

    // a copy constructor is essential for this class
    String(const String& right) : len(0), buffer(nullptr)
    {
        String temp(right.buffer);
        swap(temp);
    }

    // must have a destructor to avoid leaks
    ~String()
    {
        if(buffer) // only delete if it has been allocated
            delete[] buffer;
    }

    String& operator= (const String& right)
    {
        String temp(right.buffer);
        swap(temp);
        return *this;
    }

    int length() const
    {
        return len;
    }
private:
    void swap(String& rhs)
    {
        std::swap(buffer, rhs.buffer);
        std::swap(len, rhs.len);
    }
    char* buffer;
    int len;
};

Memory Management : arrays and dynamic allocation with new operator

4 Answers4