Memory Management : character arrays and = operator

Question

Q. In terms of Memory Management, What error would you have with the following code?

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)
{
  int n = right.length();
  for (int i = 0; i <= n; i++)
    buffer[i] = right.buffer[i];
  return *this;
}

Answer. I have no clue... Could you help me? The array size seems okay... new operator... is it because of the dangling pointer because there is no delete operator? Or rule of three?

I would appreciate your help! Please let me know. Thanks,

It does not implement the [Rule of Three](http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)), which means its a seg-fault waiting to happen. The assignment operator is also quite moronic in its implementation. The constructor, not quite as bad, but still no gem. Finally, no destructor, which means its a memory leak factory on top of everything else wrong with it. — WhozCraig, Apr 23 '13 at 05:40
The `operator=` is a buffer overflow just begging to happen, and there is no destructor nor copy assignment operator... — Yuushi, Apr 23 '13 at 05:42
Besides the obvious really bad things, there's no need for the manual calculation of the string length in the constructor, or for the manual copying. — Some programmer dude, Apr 23 '13 at 05:45

Jesse Moreland · Answer 1 · 2013-04-23T05:57:42.770

0

Is all I can suggest

~String()
{
    if(buffer)
        delete [] buffer;
    len = 0;
}

String(int length)
{
  buffer = new char[length];
  len = length;
}

String String::operator = (String rhs)
{
    if(this != &rhs)
    {
        delete [] buffer;
        buffer = new char[strlen(rhs.m_str) + 1];
        strcpy(buffer, rhs.buffer);
    }

    return *this;
}

If you don't have a destructor, every time you new the buffer in your class, it will be a memory leak because your code has no way of getting rid of it.

edited Apr 23 '13 at 05:57

answered Apr 23 '13 at 05:45

Jesse Moreland

441
6
17

You don't need to check for `NULL` when using `delete`. Your `operator=` should take a `const String&` and return a `String&`. For exception safety, always allocate new space before you `delete` (as `new` can throw - this will leave the objects in a consistent state if it does). – Yuushi Apr 23 '13 at 06:09

Memory Management : character arrays and = operator

1 Answers1