- Dillo-dev - lists.mailman3.com

dw::Image objects leak their mapKey

by jcid＠dillo.org

On Wed, Jan 28, 2009 at 07:20:38PM +0000, Jeremy Henty wrote: > > This shows up under valgrind as a leak when the HTML includes an <img > usemap="...">. Patch attached. In the second part of the patch, Why not: void Image::setUseMap (ImageMapsList *list, object::Object *key) { mapList = list; + if (mapKey && mapKey != key) + delete mapKey; mapKey = key; } > # HG changeset patch > # User Jeremy Henty <onepoint(a)starurchin.org> > # Date 1233170119 0 > # Node ID 0f662866abb4ab30f50a2da66432eff7fda8ec7a > # Parent ff6f4530c0f4dc880503d73faf5d2d828d577678 > [mq]: fix-images-leak-mapkey > > diff --git a/dw/image.cc b/dw/image.cc > --- a/dw/image.cc > +++ b/dw/image.cc > @@ -140,6 +140,8 @@ > delete altText; > if (buffer) > buffer->unref (); > + if (mapKey) > + delete mapKey; > } > > void Image::sizeRequestImpl (core::Requisition *requisition) > @@ -401,7 +403,12 @@ > void Image::setUseMap (ImageMapsList *list, object::Object *key) > { > mapList = list; > - mapKey = key; > + if (key != mapKey) { > + object::Object *oldKey = mapKey; > + mapKey = key; > + if (oldKey) > + delete oldKey; > + } > } > > } // namespace dw -- Cheers Jorge.- ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________

Jan. 28, 2009

dw::Image objects leak their mapKey

by onepoint＠starurchin.org

This shows up under valgrind as a leak when the HTML includes an <img usemap="...">. Patch attached. Regards, Jeremy Henty

Jan. 28, 2009

CSS leaks: WAS Re: [Dillo-dev] <ol>'s leak the labels of their items

by onepoint＠starurchin.org

On Tue, Jan 27, 2009 at 09:08:56AM +0100, Hofmann Johannes wrote: > On Tue, Jan 27, 2009 at 07:45:42AM +0000, Jeremy Henty wrote: > > I thought the problem was that CssPropertyList::set() does not > > free any existing value but I'm not sure I understand how these > > classes are meant to manage their memory. > > CSS values are stored in CssProperty::Value, which is a union. The > fix is to free Value.strVal, whenever Value contains a strVal (and > not an enum or length). To know this we could depend on the > corresponding CssProperty::Name (freeing it for > CSS_PROPERTY_FONT_FAMILY, but not for others), or add a Type member. > The latter is cleaner and would not need more memory, as Name and > Type could fit in two shorts. So, which should it be? I favour the former as I think that if we want to keep the Value struct lean then it is nicer to do it by not adding fields rather than resorting to packing tricks. But that's only a mild preference: either seems OK. Jeremy Henty

Jan. 28, 2009

Tools button committed

by jcid＠dillo.org

On Tue, Jan 27, 2009 at 10:47:19PM +0100, Hofmann Johannes wrote: > On Tue, Jan 27, 2009 at 06:38:50PM -0300, Jorge Arellano Cid wrote: > > Hi, > > > > The tools button changeset was just committed. > > > > Please notice the options are global. It's quite useful as is > > and making per bw options seems not worth. > > > > Go check! :-) > > Hey, pretty cool! There's more stuff that should go into the > tool menu: > > load images > increase font size > decrease font size Yes, it's a good place to experiment. > PS: Where did you get the nice icon? Got a free one from internet, and tweaked it to match the style. Good you like it! It took long to polish it. ;) -- Cheers Jorge.- ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________

Jan. 27, 2009

Tools button committed

by Johannes.Hofmann＠gmx.de

On Tue, Jan 27, 2009 at 06:38:50PM -0300, Jorge Arellano Cid wrote: > Hi, > > The tools button changeset was just committed. > > Please notice the options are global. It's quite useful as is > and making per bw options seems not worth. > > Go check! :-) Hey, pretty cool! There's more stuff that should go into the tool menu: load images increase font size decrease font size Cheers, Johannes PS: Where did you get the nice icon?

Jan. 27, 2009

Tools button committed

by jcid＠dillo.org

Hi, The tools button changeset was just committed. Please notice the options are global. It's quite useful as is and making per bw options seems not worth. Go check! :-) -- Cheers Jorge.- ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________

Jan. 27, 2009

CSS leaks: WAS Re: [Dillo-dev] <ol>'s leak the labels of their items

by Johannes.Hofmann＠gmx.de

On Tue, Jan 27, 2009 at 07:45:42AM +0000, Jeremy Henty wrote: > On Mon, Jan 26, 2009 at 11:33:43PM +0100, Hofmann Johannes wrote: > > > There is at least one leak I know of. It's font-family names that > > are parsed in CSS style data. > > Aha! I looked at the Valgrind log for that one but didn't know the > cause. Valgrind identifies the dStrdup() in > > val->strVal = dStrdup(parser->tval); > > in Css_parse_value() as the leak. What's the fix? I thought the > problem was that CssPropertyList::set() does not free any existing > value but I'm not sure I understand how these classes are meant to > manage their memory. Not 100% sure either :-). The problem only exists for string values. Most CSS values are just enums or lengths. Actually I only know of font names atm that are strings. CSS values are stored in CssProperty::Value, which is a union. The fix is to free Value.strVal, whenever Value contains a strVal (and not an enum or length). To know this we could depend on the corresponding CssProperty::Name (freeing it for CSS_PROPERTY_FONT_FAMILY, but not for others), or add a Type member. The latter is cleaner and would not need more memory, as Name and Type could fit in two shorts. We also need a way to accommodate the special "inherit" value, which is not supported yet. Regards, Johannes

Jan. 27, 2009

CSS leaks: WAS Re: [Dillo-dev] <ol>'s leak the labels of their items

by onepoint＠starurchin.org

On Mon, Jan 26, 2009 at 11:33:43PM +0100, Hofmann Johannes wrote: > There is at least one leak I know of. It's font-family names that > are parsed in CSS style data. Aha! I looked at the Valgrind log for that one but didn't know the cause. Valgrind identifies the dStrdup() in val->strVal = dStrdup(parser->tval); in Css_parse_value() as the leak. What's the fix? I thought the problem was that CssPropertyList::set() does not free any existing value but I'm not sure I understand how these classes are meant to manage their memory. Regards, Jeremy Henty

Jan. 27, 2009

<ol>'s leak the labels of their items

by Johannes.Hofmann＠gmx.de

On Mon, Jan 26, 2009 at 10:32:42PM +0000, Jeremy Henty wrote: > On Mon, Jan 26, 2009 at 11:03:20PM +0100, Hofmann Johannes wrote: > > On Mon, Jan 26, 2009 at 09:22:03PM +0000, Jeremy Henty wrote: > > > > > > Valgrind reports that <ol>s leak the labels of their items: each > > > of the first 9 <li>s leaks 3 bytes, then they leak 4 bytes per > > > <li> etc. The leaked memory is created by the line > > > > > > list_item->initWithText (dStrdup(buf), wordStyle); > > > > Ah right. The strdup() can just be dropped. Allocation and > > deallocation of text is done in Textblock itself for quite some time > > now. Please check current tip. > > It's fixed! (/me deletes valgrind suppression file) good > > But..., valgrind still reports some "indirectly lost" blocks despite > reporting no "definitely lost" or "possibly lost" blocks. How is that > possible? I am suppressing some leaks that come from the depths of X > windows and other libraries, could that be the reason? (Of course > this may have nothing to do with Dillo.) There is at least one leak I know of. It's font-family names that are parsed in CSS style data. Regards, Johannes

Jan. 26, 2009

<ol>'s leak the labels of their items

by onepoint＠starurchin.org

On Mon, Jan 26, 2009 at 11:03:20PM +0100, Hofmann Johannes wrote: > On Mon, Jan 26, 2009 at 09:22:03PM +0000, Jeremy Henty wrote: > > > > Valgrind reports that <ol>s leak the labels of their items: each > > of the first 9 <li>s leaks 3 bytes, then they leak 4 bytes per > > <li> etc. The leaked memory is created by the line > > > > list_item->initWithText (dStrdup(buf), wordStyle); > > Ah right. The strdup() can just be dropped. Allocation and > deallocation of text is done in Textblock itself for quite some time > now. Please check current tip. It's fixed! (/me deletes valgrind suppression file) But..., valgrind still reports some "indirectly lost" blocks despite reporting no "definitely lost" or "possibly lost" blocks. How is that possible? I am suppressing some leaks that come from the depths of X windows and other libraries, could that be the reason? (Of course this may have nothing to do with Dillo.) Regards, Jeremy Henty

Jan. 26, 2009