[poppler] [PATCH] Memory leaks

[Albert Astals Cid]

A Dimarts 09 Maig 2006 19:27, Carlos Garcia Campos va escriure:
> El mar, 09-05-2006 a las 15:44 +0200, Carlos Garcia Campos escribió:
> > El lun, 08-05-2006 a las 21:40 +0200, Carlos Garcia Campos escribió:
> > > El lun, 08-05-2006 a las 20:53 +0200, Albert Astals Cid escribió:
> > > > A Dilluns 08 Maig 2006 20:40, Carlos Garcia Campos va escriure:
> > > > > El lun, 08-05-2006 a las 19:56 +0200, Albert Astals Cid escribió:
> > > > > > A Dilluns 08 Maig 2006 19:11, Carlos Garcia Campos va escriure:
> > > > > > > Hi all,
> > > > > >
> > > > > > Hi
> > > > > >
> > > > > > > I've been playing with valgrind for a while with evince and
> > > > > > > I've caught several leaks in poppler.
> > > > > > >
> > > > > > > Here is a patch that fixes some leaks:
> > > > > > >
> > > > > > > http://carlosgc.linups.org/files/poppler-memleaks.diff
> > > > > >
> > > > > > i omit the glib patches as there's better people to comment.
> > > > > >
> > > > > > i'd say Catalog.cc patch is wrong, if you delete the Cstring of a
> > > > > > GooString the GooString ends up with the inside data dead as what
> > > > > > getCString returns is not a copy of the data is the data itself.
> > > > >
> > > > > it isn't a GooString but a UGooString which returns a new allocated
> > > > > string.
> > > > >
> > > > > char *UGooString::getCString() const
> > > > > {
> > > > >   char *res = new char[length + 1];
> > > > >   for (int i = 0; i < length; i++) res[i] = s[i];
> > > > >   res[length] = '\0';
> > > > >   return res;
> > > > > }
> > > >
> > > > Ahhhh, my fault, thanks, fixing it.
> > > >
> > > > > > in PDFDoc.cc is there really any possibility of reaching setup()
> > > > > > with xref not beign NULL?
> > > > >
> > > > > I'm not sure about it, I think I got these errors in valgrind
> > > > > because of an evince bug, so maybe we can avoid this part of the
> > > > > patch.
> > > > >
> > > > > > Also i disagree we should refcount GlobalParams from within the
> > > > > > lib, because who creates now the globalparams now? Not the lib,
> > > > > > but the lib decides to delete globaparams when it thinks its no
> > > > > > more needed? Nahhh, bad idea.
> > > > >
> > > > > we are creating and freeing GlobalParams in
> > > > > glib/poppler-document.cc. Without the patch we are allocating a new
> > > > > GlobalParams object for each document we load and leaking the
> > > > > previous object.
> > > >
> > > > The fix it in glib frontend then, glib frontend is not the only one
> > > > you know ;-)
> > >
> > > Yeah, you are right, I'll fix it by wrapping globalParams in glib
> > > frontend.
> > >
> > > > Albert
> > > >
> > > > > > poppler/TextOutputDev.cc is for krh.
> > > > > >
> > > > > > So no patch i have enough karma/mood to commit ;-)
> > > > > >
> > > > > > Albert
> >
> > Ok, here is an updated patch. I've kept the PDFDoc part because I'm not
> > sure if it's a evince or poppler bug. Since setup method could be called
> > more than once after object creation, I think these checks don't hurt.

Nah, it can not, it's a private member and only called from PDFDoc 
constructors so you can only call setup exactly once, no?

Albert

> > krh, I've wrapped the globalParamas in the glib frontend by using an
> > object PopplerGlobalParams. The code is in the poppler-document.cc
> > instead of having its own file because this object is totally private to
> > poppler-document.
>
> I'm wrong here. poppler-page also needs a reference to globalParams so
> I've reworked the patch again:
>
> http://carlosgc.linups.org/files/poppler-memleaks3.diff

	
	
		
______________________________________________ 
LLama Gratis a cualquier PC del Mundo. 
Llamadas a fijos y móviles desde 1 céntimo por minuto. 
http://es.voice.yahoo.com