is the following new overload leaking memory?

Question

I have encountered the following code:

    class a {
    public:

        void *  operator new(size_t l, int nb);
        double  values;
    };
void *a::operator new (size_t l,int n)
{
    return new char[l+ (n>1 ? n - 1 : 0)*sizeof(double)];
}

From what I get it is then used to have an array like structure that start at "values":

double* Val = &(p->a->values) + fColumnNumber;

My question is : is there a memory leak? I am very new to overloading new operator, but I'm pretty sure that the memory allocated is not deallocated properly. Also does that mean I can never create a "a" class on the stack?

thanks

Answer 1

1. You can definitely create Class "a" on the stack.

2. There are 4 (actually more but will stick with the basics) new & delete method signatures you should know.

   void* operator new (std::size_t size) throw (std::bad_alloc);
   void* operator new[] (std::size_t size) throw (std::bad_alloc);
   void operator delete (void* ptr) throw ();
   void operator delete[] (void* ptr) throw ();
   

   You are allocating an array within the "operator new" method which should be done in the "operator new[]" method. This will get rid of your nasty check. Write both, "operator new" and "operator new[]"

3. Don't forget you want to give the caller an object of type "a" ( a myA = new a) so make sure you return "a" not char*, therefore you need to also be casting.

4. You need to write the corresponding delete[] and delete methods.

5. To answer your question, I believe it does leak memory. The new signature you've provided is called the "placement new". This allows you to allocate a new pointer without allocating memory but to give it a location to point to. Example: If you needed a pointer to a specific point in memory.

   long z = 0x0F9877F80078;
   a myA = new (z) a[5];  // 5 pointers that point to 0x0F9877F80078
   

By definition the placement-new operator is not supposed to allocate memory and since you have you make be leaking. Get rid of your second argument, which you can do now since you have 2 versions of operator new and you're good to go. Don't forget to return an object "a".

Check out IBM's Info Center: http://publib.boulder.ibm.com/infocenter/comphelp/v8v101/index.jsp?topic=%2Fcom.ibm.xlcpp8a.doc%2Flanguage%2Fref%2Fcplr318.htm

And the reference or references, cpluplus.com: http://www.cplusplus.com/reference/std/new

Answer 2

It's fairly easy to find out. Write a loop that constructs and deconstructs lots of a's, and watch your memory usage. It'll go up pretty fast if it's leaking.

Answer 3

I believe it technically produces UB as it is, though it's a form of UB that will probably never cause a visible side effect (it's using new [], but I believe that'll get matched up with delete -- but for char, this usually won't cause a visible problem).

IMO, it's almost worse that it's using a new expression to allocate what should really be raw bytes instead of objects. If I were doing it, I'd write it like:

void *a::operator new (size_t l,int n)
{
    return ::operator new(l+ (n>1 ? n - 1 : 0)*sizeof(double));
}

You'd match that up with:

void a::operator delete(void *block)
{
    ::operator delete(block);
}

Answer 4

Its fine as it is, but you'd need to use delete[], not delete, from the code that uses this class as it allocates an array. Note that the user wouldn't get any hints that they need to do this - so overloading a delete operator for them would be a good idea.

Answer 5

I don't see why the default operator delete called on an a * wouldn't be able to correctly deallocate the memory allocated by this custom operator new. The best way to check would be to actually write some code and find out, although rather than rob05c's technique I'd probably run it in a profiler such as valgrind. I assume the questioner sees a memory leak happening and suspects this as the cause, so writing a test case around this operator seems like a worthwhile endeavour.

Obviously it will leak if nobody gets around to actually deleting it afterwards...

I'd question the necessity of overriding new for this kind of functionality, but I also assume this was somebody else's code.