Yes,

but no. wmaker crash when you type Alt+Tab. Probably the patch is good, but 
something is wrong in other place.

Cheers,
kix


On Thu, 15 Nov 2012, Christophe escribió:

> 
> ----- Rodolfo García Peñas <[email protected]> a écrit :
> > Hi,
> > 
> > don't apply this patch. It causes wmaker crash using alt+tab.
> 
> Hi Rodolfo,
> 
> I had a look at the code, I think I can propose a patch to fix that. Thanks 
> for pointing the issue, you must be quite motivated to try Valgrind!
> 
> Best regards,
> Christophe.
> 
> 
> > 
> > Cheers,
> > kix
> > 
> > On Tue, 06 Nov 2012, Rodolfo kix Garcia escribió:
> > 
> > > 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
> > > 
> > 
> > 
> > -- 
> > ||// //\\// Rodolfo "kix" Garcia
> > ||\\// //\\ http://www.kix.es/
> > 
> > 
> > -- 
> > To unsubscribe, send mail to [email protected].
> 
> 
> --
> To unsubscribe, send mail to [email protected].

-- 
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/


-- 
To unsubscribe, send mail to [email protected].

Reply via email to