My memory is not free'd

c++, memory-leaks

Solution

I don't think you are representing pixels correctly in your image class. I think you can use a simple 1-dimensional `vector` of the correct unsigned type (`uint32_t`?).

(in header):

class Image {
protected:
    std::vector<uint32_t> pixels;
    ...
};

(in implementation file)

size_t Image::offset(unsigned x, unsigned y) {
    return (y * width) + x;
}

Image::Image(const SDL_Surface* surface)
{
    width = surface->w;
    height = surface->h;
    pixels.reserve(width * height);
    for(unsigned x = 0; x < width; x++)
        for(unsigned y = 0; y < height; y++)
            pixels[offset(x, y)] = getPixelFromSDLSurface(surface, x, y);
}

Your destructor would then be completely empty, as there is nothing for it to do:

Image::~Image() {
}

Note you need to use the `offset()` method to get the correct index into the `vector` for any given x/y pair.

Problem

I have a rather large program that generally runs wonderfully, but uses insane amounts of memory to run. This is a machine learning approach that collects a lot of data, and thus that is generally okay, but the memory grows and grows very fast even after all the data is collected, and so I used valgrind massif to find out what's going wrong. The top of massif's heap tree looks like this: ``` 99.52% (60,066,179B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc. ->43.50% (26,256,000B) 0x439785: Image::Image(SDL_Surface*) (Image.cpp:95) | ->43.50% (26,256,000B) 0x437277: EncodedFeature::forwardPass() (EncodedFeature.cpp:65) .... ``` So I thought, hmm, maybe the constructed Image is not free'd, but no: ``` void EncodedFeature::forwardPass() { // Get image: Image* img = new Image(screen); // Preprocess: if(preprocessor) preprocessor->process(img); // Do forward pass: encoder->encode(img, features); delete img; } ``` So to the Image constructor: ``` Image::Image(SDL_Surface* surface) { this->width = surface->w; this->height = surface->h; pixels = new int*[width]; for(int i = 0; i < width; i++) pixels[i] = new int[height]; for(int x = 0; x < surface->w; x++) for(int y = 0; y < surface->h; y++) pixels[x][y] = getPixelFromSDLSurface(surface, x, y); } ``` Just allocates a pixel array that gets freed in the destructor later on: ``` Image::~Image() { if(!pixels) return; for(int x = 0 ; x < width; x++) delete[] pixels[x]; delete[] pixels; } ``` So the last culprit: ``` Uint32 Image::getPixelFromSDLSurface(SDL_Surface *surface, int x, int y) { if(!surface) return 0; // Got this method from http://www.libsdl.org/cgi/docwiki.fcg/Pixel_Access int bpp = surface->format->BytesPerPixel; /* Here p is the address to the pixel we want to retrieve */ Uint8 *p = (Uint8 *)surface->pixels + y * surface->pitch + x * bpp; switch(bpp) { case 1: return *p; break; case 2: return *(Uint16 *)p; break; case 3: if(SDL_BYTEORDER == SDL_BIG_ENDIAN) return p[0] << 16 | p[1] << 8 | p[2]; else return p[0] | p[1] << 8 | p[2] << 16; break; case 4: return *(Uint32 *)p; break; default: return 0; /* shouldn't happen, but avoids warnings */ } } ``` As mentioned in the comment, I got that one from the SDL wiki, so I hope there is nothing leaking there. bpp in my case is actually always 1, so it just returns the int at address p, which doesn't sound leaky to me. I'm at the end of my wits here. Can anyone think of where the memory went? I mean, massif points specifically to the Image constructor, but I can't see anything wrong there... Thanks a lot for looking at my problem! Max Answering your comments: You are right, I did not need img as a pointer. I come from a Java background so I just want everything to be pointers :) Changed it but didn't help. Line 95 is inside the first for loop of the constructor: `pixels[i] = new int[height];` In one of the preprocessors I do resize the image, but I do this calling my reset function, which should make sure that the old array is deleted: ``` void Image::reset(int width, int height) { if(pixels) { // Delete old image: for(int x = 0 ; x < width; x++) delete[] pixels[x]; delete[] pixels; } this->width = width; this->height = height; pixels = new int*[width]; for(int i = 0; i < width; i++) pixels[i] = new int[height]; } ``` After which I refill the pixel values... No exceptions are being thrown anywhere. Where would you suggest I use smart pointers? Thanks for all the answers!

Original source