Memory not being freed, causing giant memory leak

Unfortunately the solutions have not worked yet; setting result.values pointer to 0 doesn't actually relieve the memory usage. I've also tried free(result.values) in place of that, but that is not desired as that deletes my string.

Edit 2: I'll try writing a stack destructor.

Edit 3: Gotcha. Thanks DeadMG, writing a destructor that free(values) did the trick perfectly! Wow... so simple.

In my Unicode library for C++, the ustring class has operator= functions set for char* values and other ustring values. When doing the simple memory leak test:

#include <cstdio>
#include "ucpp"
main() {
  ustring a;
  for(;;)a="MEMORY";
}

the memory used by the program grows uncontrollably (characteristic of a program with a big memory leak) even though I've added free() calls to both of the functions. I am unsure why this is ineffective (am I missing free() calls in other places?)

This is the current library code:

#include <cstdlib>
#include <cstring>
class ustring {
  int * values;
  long len;
  public:
  long length() {
    return len;
  }
  ustring() {
    len = 0;
    values = (int *) malloc(0);
  }
  ustring(const ustring &input) {
    len = input.len;
    values = (int *) malloc(sizeof(int) * len);
    for (long i = 0; i < len; i++)
      values[i] = input.values[i];
  }
  ustring operator=(ustring input) {
    ustring result(input);
    free(values);
    len = input.len;
    values = input.values;
    return * this;
  }
  ustring(const char * input) {
    values = (int *) malloc(0);
    long s = 0;                                                                 // s = number of parsed chars
    int a, b, c, d, contNeed = 0, cont = 0;
    for (long i = 0; input[i]; i++)
      if (input[i] < 0x80) {                                                    // ASCII, direct copy (00-7f)
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = input[i];
      } else if (input[i] < 0xc0) {                                             // this is a continuation (80-bf)
        if (cont == contNeed) {                                                 // no need for continuation, use U+fffd
          values = (int *) realloc(values, sizeof(int) * ++s);
          values[s - 1] = 0xfffd;
        }
        cont = cont + 1;
        values[s - 1] = values[s - 1] | ((input[i] & 0x3f) << ((contNeed - cont) * 6));
        if (cont == contNeed) cont = contNeed = 0;
      } else if (input[i] < 0xc2) {                                             // invalid byte, use U+fffd (c0-c1)
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = 0xfffd;
      } else if (input[i] < 0xe0) {                                             // start of 2-byte sequence (c2-df)
        contNeed = 1;
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = (input[i] & 0x1f) << 6;
      } else if (input[i] < 0xf0) {                                             // start of 3-byte sequence (e0-ef)
        contNeed = 2;
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = (input[i] & 0x0f) << 12;
      } else if (input[i] < 0xf5) {                                             // start of 4-byte sequence (f0-f4)
        contNeed = 3;
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = (input[i] & 0x07) << 18;
      } else {                                                                  // restricted or invalid (f5-ff)
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = 0xfffd;
      }
    len = s;
  }
  ustring operator=(const char * input) {
    ustring result(input);
    free(values);
    len = result.len;
    values = result.values;
    return * this;
  }
  ustring operator+(ustring input) {
    ustring result;
    result.len = len + input.len;
    result.values = (int *) malloc(sizeof(int) * result.len);
    for (lo开发者_开发百科ng i = 0; i < len; i++)
      result.values[i] = values[i];
    for (long i = 0; i < input.len; i++)
      result.values[i + len] = input.values[i];
    return result;
  }
  ustring operator[](long index) {
    ustring result;
    result.len = 1;
    result.values = (int *) malloc(sizeof(int));
    result.values[0] = values[index];
    return result;
  }
  operator char * () {
    return this -> encode();
  }
  char * encode() {
    char * r = (char *) malloc(0);
    long s = 0;
    for (long i = 0; i < len; i++) {
      if (values[i] < 0x80)
        r = (char *) realloc(r, s + 1),
        r[s + 0] = char(values[i]),
        s += 1;
      else if (values[i] < 0x800)
        r = (char *) realloc(r, s + 2),
        r[s + 0] = char(values[i] >> 6 | 0x60),
        r[s + 1] = char(values[i] & 0x3f | 0x80),
        s += 2;
      else if (values[i] < 0x10000)
        r = (char *) realloc(r, s + 3),
        r[s + 0] = char(values[i] >> 12 | 0xe0),
        r[s + 1] = char(values[i] >> 6 & 0x3f | 0x80),
        r[s + 2] = char(values[i] & 0x3f | 0x80),
        s += 3;
      else
        r = (char *) realloc(r, s + 4),
        r[s + 0] = char(values[i] >> 18 | 0xf0),
        r[s + 1] = char(values[i] >> 12 & 0x3f | 0x80),
        r[s + 2] = char(values[i] >> 6 & 0x3f | 0x80),
        r[s + 3] = char(values[i] & 0x3f | 0x80),
        s += 4;
    }
    return r;
  }
};

---

Where's the destructor of ustring? Shouldn't it free the allocated memory?

---

Let's look e.g. at the assignment operator:

ustring operator=(ustring input)
{
    ustring result(input);
    ...
    return *this;
}

You pass a ustring parameter by value. This will likely result in a copy being created via the copy constructor. You invoke copy construction another time to initialize result. I suspect that you invoke copy construction yet another time when you return *this as a ustring (again by value, instead of by reference).

Let's look at one of these three cases, namely result: When this local variable goes out of scope (ie. at the end of the function), it will be automatically destroyed. But if you don't provide a destructor (~ustring) that frees the allocated memory, you'll get a memory leak.

And since you apparently have lots of copy construction and pass-by-value going on without having a destructor for freeing the allocated memory, you'll get lots and lots and lots of unfreed memory.

---

Besides: Why do you use malloc and free instead of new[] and delete[]? You could get rid of the ugly sizeof(int) * ... calculations and (int*) type casts. Instead of:

values = (int *) malloc(sizeof(int) * len);

you'd simply write:

values = new int[len];

---

Basically, you create far too many ustrings, you need a LOT more references, and you didn't implement a destructor so when they all fall off the stack, they don't get freed.

Also, when in your assignment operator, you need to set result.values to NULL, else the memory will be deleted. You could use a move operator to make this a nice fast operation, although I still don't understand why you would.

---

  ustring operator=(ustring input) {
    ustring result(input);
    free(values);
    len = input.len;
    values = input.values;
    return * this;
  }

Why is result being declared in this?

---

As soon as You implement the destructor, this implementation of assignment will bite You

  ustring operator=(const char * input) {
    ustring result(input);
    free(values);
    len = result.len;
    values = result.values;
    return * this;
  }

No, it is not an argument for leaving the destructor not implemented.

EDIT:

ustring object owns an array allocated with malloc and should free that memory when it gets destroyed.

If you create a local object

    ustring result(input);

It will get destroyed when the function returns.

In the line

    values = result.values;

You copy the pointer from Your local object - result, but result doesn't know about it, so it should destroy the array and leave *this with a dangling pointer. You have to change the state of result in a way, that prevents it from freeing the memory, because You took the ownership of the array from it. One way to do that could be setting it's pointer to null. Calling free on a null pointer is legal, so You don't have to worry about an attempt to free memory by result's destructor.

ustring& operator=(const char * input) {
    ustring result(input);
    free(values);
    len = result.len;
    values = result.values;
    result.values = 0;
    return * this;
}

---

1. Try using RAII-techniques to avoid problems with memory leaks. If you wrap your arrays in something like a boost::shared_array you will not have to write an explicit destructor to free the memory.
2. ustring operator=(ustring input) should be ustring operator=(const ustring & input) to avoid a copy of the argument. This should always be done if you pass a non-integral type as read-only parameter.
3. Using shared_array are something like it also gets rid of the problem of calculating size.

Just some hints. On another note, your code is incredibly hard to read and understand. You should really do something about that if you expect other persons to ever work with that. Make your names more descriptive, refactor recurring code into functions that express what it does. For instance instead of using a for-loop to copy your values, you could use std::copy. You may also consider to replace all your magic numbers (e.g. 0x1f) by constants whose names express the semantics of the value.