Memory Leak in this code?

Question

Sample here:

class A
{
private:
    int buff[1000];
public:
    A(int n)
    {
        buff = new int[n];
    }
};

 int main()
 {
     for (int i = 10; i < 1000; i++)
     {
         A a(i);
     }
     return 0;
 }

Can anyone help me? what's wrong with this code? Memory Leak? Or any other errors?

Yes, obviously. You have a `new` without a matching `delete`. — Kerrek SB, Dec 10 '14 at 22:21
A memory leak should be the least of your worries. This code doesn't even compile. — David G, Dec 10 '14 at 22:21

score 1 · Accepted Answer · edited Dec 10 '14 at 22:27

1

Aside from the issues pointed out in the comments, you should create a destructor which calls delete[] on the item you allocated on the heap (using the new keyword):

class A{
   private:
       int buff[1000];
   public:
       A(int n){
           buff=new int[n];
       }
       ~A() {
           delete[] buff;
       }
};

int main(){
    for(int i=10;i<1000;i++){
        A a(i);
     }   
     return 0;
}

Don't forget the [] after the delete because it's an array.

edited Dec 10 '14 at 22:27

Kijewski

25,517
12
101
143

answered Dec 10 '14 at 22:25

tigertrussell

521
2
13

3

That still won't compile. Arrays aren't assignable. – David G Dec 10 '14 at 22:27
@0x499602D2, well, tigerrussell says "side from the issues pointed out in the comments" ... – Kijewski Dec 10 '14 at 22:28
@Kay There are no other specific issues pointed out in the comments. The OP didn't know that arrays aren't assignable. – David G Dec 10 '14 at 22:29
Change that `int buff[1000];` to `int* buff;` and it'd be good. Not great, but at least valid. – Fred Larson Dec 10 '14 at 22:30
1

Well, I really did mean "aside from the fact it won't compile." The only thing I cared about demonstrating was the use of destructors and `delete[]`. OP can figure the rest out - s/he is an adult.. maybe. – tigertrussell Dec 10 '14 at 22:37
By the way OP you should check out http://codepad.org/ for typing up simple examples -- it will help you avoid scenarios where the code you posted won't compile. – tigertrussell Dec 10 '14 at 22:38
That fixes the memory leak, but gives it invalid copy semantics: it will explode horribly if you accidentally copy an object. http://stackoverflow.com/questions/4172722 – Mike Seymour Dec 11 '14 at 00:13

score 0 · Answer 2 · answered Dec 11 '14 at 01:33

Modern C++ says you should be using new or delete in your code at all, but rather std::unique_ptr which does your memory management for you.

As such, your class would fit modern idioms better in this way:

#include <memory>

class A
{
private:
    std::unique_ptr<int[]> buff;
public:
    A(int n)
        : buff(std::make_unique(int[n])) // buff(new int[n]) if using C++11
    { }
};

int main()
{
    A a(5);
    return 0;
}

Unfortunately std::make_unique isn't available in C++11, so until then you have to use new. However, you can still use std::unique_ptr, which will solve your memory leak issue.

Memory Leak in this code?

2 Answers2