CODEWITHSUNDEEP
c++memory-leaks
user2590664
user2590664

Reputation: 51

How can this delete to prevent the memory leak?

I really really thank the people who give me the help.Thank you!

struct MyImage
{
    BYTE* pImage;
    int width;
    int heigth;
};
MyImage* pMyImage = new MyImage;
pMyImage->pImage = new BYTE[width * heigth];

should I do like this?

delete [] pMyImage->pImage;

or should I do like this?

delete[] pMyImage->pImage;
delete pMyImage;

hope for your ideas and Thanks.

MyImage* transform(Bitmap &gdiImage)
{
MyImage* image=new MyImage;//新建一个MyImage

int height=gdiImage.GetHeight();
int width=gdiImage.GetWidth();

image->pImage=new BYTE[height*width];//为存储灰度图像数据分配内存

image->height=height;
image->width=width;

Color temp;
for(int y = 0;y < height; ++y)
    for(int x = 0;x < width; ++x)
    {
        //获取当前GDI+图像坐标所指向的像素的颜色
        gdiImage.GetPixel(x, y, &temp);
        //将这个像素的灰度值赋给灰度图像对应的内存中的相应的字节
        *(image->pImage + y * width + x) = transformPixel(temp.GetValue());
    }
return image;
}

My code is as below, this function translate a gdiImage to a struct MyImage. as a firend say, as follows, I can not new MyImage and new pImage,the element of MyImage. what should I do? thank you

Upvotes: 3

Views: 169

Answers (5)

Pierre Fourgeaud
Pierre Fourgeaud

Reputation: 14510

Your second option is the good one :

delete[] pMyImage->pImage;
delete pMyImage;

A new must always be matched with a delete and a new[] must always be matched with a delete[].

MyImage should be responsible for allocating/freeing this data to prevent any error...

You could place the delete[] pMyImage->pImage; in the MyImage destructor :

MyImage::~MyImage()
{
    delete [] pImage;
}

Finally, I suggest you to take a look to the smart pointers in c++. For example to std::unique_ptr :

The object is destroyed and its memory deallocated when either of the following happens:

  • unique_ptr managing the object is destroyed
  • unique_ptr managing the object is assigned another pointer via operator=() or reset().

The object is destroyed using a potentially user-supplied deleter by calling Deleter(ptr). The deleter calls the destructor of the object and dispenses the memory.

Something like that :

#include <memory>

struct MyImage
{
    std::unique_ptr<BYTE[]> m_Image;
    int width;
    int heigth;
};

MyImage* pMyImage = new MyImage;
pMyImage->m_Image= std::unique_ptr<BYTE[]>( new BYTE[ width * height ] );

Upvotes: 3

Karadur
Karadur

Reputation: 1246

I'd implement constructor and destructor in MyImage:

struct MyImage
{
    MyImage() : pImage(nullptr), width(0), height(0)
    {
    }

    ~MyImage()
    {
        delete [] pImage;
    }
    BYTE* pImage;
    int width;
    int heigth;
};

Generally, it's better to be made a class where pImage, width and height are private members.

Upvotes: -1

Frerich Raabe
Frerich Raabe

Reputation: 94329

Your second alternative would be correct, but to avoid that you can get calling delete (e.g. forgetting to do so) wrong in the first place, I suggest to use appropriate tools which take care of that.

For instance, chances are that your dynamically allocated BYTE array might be a good candidate for a std::vector. The MyWidget could be managed by some kind of smart pointer, e.g. std::unique_ptr or std::shared_ptr or the like. Or maybe you don't even need to allocate it dynamically at all but can just create it on the stack and then pass the address around should you need it. So, maybe something like this:

// An example function dealing with images. This one draws it.
void draw(MyImage *img);

struct MyImage
{
    std::vector<BYTE> image;
    int width;
    int heigth;
};

MyImage myImage;
myImage.image.resize(width * height);
// ...
draw(&myImage);

Upvotes: 6

maditya
maditya

Reputation: 8896

The second one. Any memory that you manually allocate with new or new[] has to be manually deallocated using delete or delete[] respectively.

Upvotes: 1

JBL
JBL

Reputation: 12907

The correct one is 

delete[] pMyImage->pImage;
delete pMyImage;

You must match a new with a delete, and a new[] with a delete[] (with a few exceptions, e.g. when you know a library handles memory itself, like Qt and its parent/child system)

Upvotes: 1

Related Questions