4

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

6
  • 2
    The code you show doesn’t deallocate memory at all. Show this code, otherwise we cannot say whether memory is leaked. I don’t see an a priori reason to suspect memory is leaked, unless operator delete is really missing. May 11, 2012 at 13:52
  • 1
    it is missing, that's all i have
    – lezebulon
    May 11, 2012 at 13:55
  • Yes, you'll need to delete anyway, overloaded operator or not. What made you think overloading new would somehow work differently?
    – Mr Lister
    May 11, 2012 at 13:56
  • Ah, that makes the question more interesting. May 11, 2012 at 13:56
  • @KonradRudolph Why does it make the question more interesting? It's now deteriorated into a simple missing destructor problem, that's all.
    – Mr Lister
    May 11, 2012 at 13:57

5 Answers 5

5

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);
}
3
  • +1 for pointing out that it's UB that doesn't hurt anyone and a good alternative. May 11, 2012 at 14:38
  • @lezebulon: Primarily that if you have an operator new, you should have a matching operator delete, if only to make it obvious that what you have is correct, and you haven't forgotten something. May 11, 2012 at 14:45
  • Thanks for the answer, what I was worried about was that only sizeof(a) bytes would be released when calling delete. I'm still not so sure why all the allocated memory will be freed
    – lezebulon
    May 11, 2012 at 19:33
1

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.

0
0

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.

1
  • That tells you almost nothing since the standard doesn’t specify how implementations track memory. May 11, 2012 at 14:29
0

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.

0
  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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.