Hi, I found (not me, valgrind ;-)) a memory leak with the calloc function. I sent a patch proposal, but I am not sure if the patch is ok. Please, if more people can check the patch...
Cheers, kix -- ||// //\\// Rodolfo "kix" Garcia ||\\// //\\ http://www.kix.es/
>From 60f7982cd9b48e5e1c8399b0470e6933abfc563f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?= <[email protected]> Date: Tue, 6 Nov 2012 22:59:13 +0100 Subject: [PATCH] calloc free memory leak The memory allocated using calloc is not properly freed. This is the valgrind error: 5 errors in context 11 of 12: Syscall param writev(vector[...]) points to uninitialised byte(s) at 0x430A65B: writev (writev.c:51) by 0x44F17C5: ??? (in /usr/lib/i386-linux-gnu/libxcb.so.1.1.0) by 0x42180AF: ??? (in /usr/lib/i386-linux-gnu/libX11.so.6.3.0) Address 0x469f475 is 421 bytes inside a block of size 16,384 alloc'd at 0x4026A68: calloc (vg_replace_malloc.c:566) by 0x410EAE9: XOpenDisplay (in /usr/lib/i386-linux-gnu/libX11.so.6.3.0) by 0x3D474E40: ??? The memory is allocated in this way: contrib = (CLIST *) calloc(new_width, sizeof(CLIST)); And is freed in this way: for (i = 0; i < dst->height; ++i) free(contrib[i].p); But dst->height can be less than the number of elements (contrib[i].n) The correct way should be: for (i = 0; i < tmp->height; i++) { for (k = 0; k < contrib[i].n; k++) free(contrib[i].p); } After this patch, valdgrind doesn't report errors. This patch also includes the variable contrib inside the function (now is not global) and some style changes. --- wrlib/scale.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/wrlib/scale.c b/wrlib/scale.c index 82ed57e..6bbf3c6 100644 --- a/wrlib/scale.c +++ b/wrlib/scale.c @@ -278,14 +278,13 @@ typedef struct { CONTRIB *p; /* pointer to list of contributions */ } CLIST; -CLIST *contrib; /* array of contribution lists */ - /* clamp the input to the specified range */ #define CLAMP(v,l,h) ((v)<(l) ? (l) : (v) > (h) ? (h) : v) /* return of calloc is not checked if NULL in the function below! */ -RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned new_height) +RImage *RSmoothScaleImage(RImage *src, unsigned new_width, unsigned new_height) { + CLIST *contrib; /* array of contribution lists */ RImage *tmp; /* intermediate image */ double xscale, yscale; /* zoom scale factors */ int i, j, k; /* loop variables */ @@ -294,8 +293,7 @@ RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned new_height) double width, fscale; /* filter calculation variables */ double rweight, gweight, bweight; RImage *dst; - unsigned char *p; - unsigned char *sp; + unsigned char *p, *sp; int sch = src->format == RRGBAFormat ? 4 : 3; dst = RCreateImage(new_width, new_height, False); @@ -332,7 +330,6 @@ RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned new_height) } } } else { - for (i = 0; i < new_width; ++i) { contrib[i].n = 0; contrib[i].p = (CONTRIB *) calloc((int) ceil(fwidth * 2 + 1), sizeof(CONTRIB)); @@ -381,9 +378,11 @@ RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned new_height) } /* free the memory allocated for horizontal filter weights */ - for (i = 0; i < tmp->width; ++i) { - free(contrib[i].p); + for (i = 0; i < tmp->width; i++) { + for (k = 0; k < contrib[i].n; k++) + free(contrib[i].p); } + free(contrib); /* pre-calculate filter contributions for a column */ @@ -475,9 +474,11 @@ RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned new_height) free(sp); /* free the memory allocated for vertical filter weights */ - for (i = 0; i < dst->height; ++i) { - free(contrib[i].p); + for (i = 0; i < tmp->height; i++) { + for (k = 0; k < contrib[i].n; k++) + free(contrib[i].p); } + free(contrib); RReleaseImage(tmp); -- 1.7.10.4
