Floodfill memory leak iPhone

Question

I'm implementing a floodfill function in C for the iPhone.

The fill works, although I'm having 2 issues.

The phone gives a memory warning after a few executions of the code below. Most likely a memory leak. Also note that the unsigned char *data (the image data) is being free()'d at the end of the floodfill.
(lesser issue) If I attempt to write RGB colors to pixels that are greater than approximately (r:200,g:200,b:200,a:200) I get weird artifacting happening. A workaround for this was to simply limit the values.

I suspect there may be a correlation between both of these problems.

The code below describes my flood-fill algorithm, using a stack:

.h:

typedef struct {
    int red;
    int green;
    int blue;
    int alpha;
} GUIColor;


struct pixel_st {
    int x;
    int y;
    struct pixel_st *nextPixel;
};
typedef struct pixel_st pixel;

.m:

   void floodFill(CGPoint location, GUIColor tc, GUIColor rc, size_t width, size_t height, unsigned char *data){
    if (isGUIColorEqual(tc, rc)) return;

    pixel* aPixel = (pixel *) malloc(sizeof (struct pixel_st));
    NSLog(@"sizeof aPixel : %i",(int)sizeof(aPixel));

    (*aPixel).x = location.x;
    (*aPixel).y = location.y;
    (*aPixel).nextPixel = NULL;

    int i = 0;

    NSLog(@"Replacement color A%i, R%i, G%i, B%i",rc.alpha,rc.red,rc.green, rc.blue);

    while (aPixel != NULL){
        pixel *oldPixel_p = aPixel;
        pixel currentPixel = *aPixel;
        aPixel = currentPixel.nextPixel;


        //Now we do some boundary checks
        if (!isOutOfBounds(currentPixel.x, currentPixel.y, width, height)){
            //Grab the current Pixel color
            GUIColor currentColor = getGUIColorFromPixelAtLocation(CGPointMake(currentPixel.x, currentPixel.y), width, height, data);

            if (isGUIColorSimilar(currentColor, tc)){
                //Colors are similar, lets continue the spread
                setGUIColorToPixelAtLocation(CGPointMake(currentPixel.x, currentPixel.y), rc, width,height, data);

                pixel *newPixel;


                if ((newPixel = (pixel*) malloc(sizeof(struct pixel_st))) != NULL) {
                    (*newPixel).x = currentPixel.x;
                    (*newPixel).y = currentPixel.y-1;
                    (*newPixel).nextPixel = aPixel;
                    aPixel = newPixel;

                }
                if ((newPixel = (pixel*) malloc(sizeof(struct pixel_st))) != NULL) {
                    (*newPixel).x = currentPixel.x;
                    (*newPixel).y = currentPixel.y+1;
                    (*newPixel).nextPixel = aPixel;
                    aPixel = newPixel;
                }
                if ((newPixel = (pixel*) malloc(sizeof(struct pixel_st))) != NULL) {
                    (*newPixel).x = currentPixel.x+1;
                    (*newPixel).y = currentPixel.y;
                    (*newPixel).nextPixel = aPixel;
                    aPixel = newPixel;
                }
                if ((newPixel = (pixel*) malloc(sizeof(struct pixel_st))) != NULL) {
                    (*newPixel).x = currentPixel.x-1;
                    (*newPixel).y = currentPixel.y;
                    (*newPixel).nextPixel = aPixel;
                    aPixel = newPixel;
                }
                free(oldPixel_p);
                i ++;
                if (i == width * height * 4 * 5) break;

            }


        }
    }

    free(aPixel);
}

This implementation of the stack is based on the ObjFloodFill found here:

https://github.com/OgreSwamp/ObjFloodFill/blob/master/src/FloodFill.m

Why ar u allocating memory once and then freeing multiple times inside while loop — hazzelnuttie, Mar 07 '13 at 10:07
The stack is based on the ObjFloodFill, I'm porting it over to my code, but I havent had much experience with straight-C memory management so I'm unsure. — Ospho, Mar 07 '13 at 10:14
On top of the page it is noted as "OgreSwamp2 years ago added comment to the ScanlineFloodfill that it doesnt work " — hazzelnuttie, Mar 07 '13 at 10:20
The ScanlineFloodfill algorithm doesnt work, but the 4-way does, I'm unsure if the memory-management in it is good though — Ospho, Mar 07 '13 at 10:22

score 1 · Answer 1 · answered Mar 07 '13 at 10:20

1

First of all, each if ((newPixel = (pixel*) malloc(... inside the loop allocates new memory block, so, you have 4 allocations inside the loop and only 1 deallocation.

Second, I can't understand why don't you simply use objects on stack? Do you really need to allocate newPixel, oldPixel and so on on the heap? Review the implementation, there might be much simpler way to implement the same also without managing the memory issues at all.

answered Mar 07 '13 at 10:20

Gobra

4,263
2
15
20

There probably is, but i'm unsure how to go about it, I was previously using a recursive function floodfill, but that would crash the device (on the iOS simulator it would work) because of stack-overflow. – Ospho Mar 07 '13 at 10:29

score 0 · Answer 2 · answered Mar 07 '13 at 10:31

You need to move the deallocation of oldPixel_p to outside the if blocks, because it is "consumed" always.

Also, the final free only frees the first element in the list. The list may have more than one element. You need to step through the list and free all remaining elements.

Floodfill memory leak iPhone

2 Answers2