- Dillo-dev - lists.mailman3.com

[patch] use delete[] where necessary

by Johannes.Hofmann＠gmx.de

On Wed, Jul 02, 2008 at 07:14:31PM -0400, Jorge Arellano Cid wrote: > On Wed, Jul 02, 2008 at 10:38:51PM +0200, Johannes Hofmann wrote: > > Hi, > > > > attached patch uses delete[] for deleting arrays as required by C++. > > Committed. > > Hey, a quick test showed memory usage reduction! Ah nice. Didn't notice that. Cheers, Johannes

July 3, 2008

[patch] iterator related bugs

by Johannes.Hofmann＠gmx.de

Hi, now I have a reliable way to reproduce the problem. On DragonFly I use debug malloc options, but valgrind should also work. Even without any debug features I can crash dillo that way, but it takes longer. * Choose a rather long page that does not fit on the screen with lot's of links. I use my bookmarks page. * Start the search dialog and enter a very common string e.g. 2 chars and hold the Enter key down. Dillo now scrolls through the page over and over again. * Now click the 1. mouse button and move the mouse like crazy over the scrolling page so arbitrary text gets selected and deselected. That crashes dillo pretty soon for me. Attached patch fixes the problem here. Please test and check for potential memory leaks. BTW, the patch fixes one leak in Iterator::scrollTo(). Cheers, Johannes

July 3, 2008

Re: patch: rm unused dw configure.in options]

by jcid＠dillo.org

On Thu, Jul 03, 2008 at 12:04:26AM +0000, corvid wrote: > Jorge accidentally applied my dw2 configure.in change (sent off-list) > to dillo2 instead, so let's put the libjpeg stuff back in case > anyone needs it: Yes, I should not commit when as tired as yesterday... Fix committed. -- Cheers Jorge.-

July 3, 2008

Re: patch: rm unused dw configure.in options]

by corvid＠lavabit.com

Jorge accidentally applied my dw2 configure.in change (sent off-list) to dillo2 instead, so let's put the libjpeg stuff back in case anyone needs it:

July 3, 2008

[patch] use delete[] where necessary

by jcid＠dillo.org

On Wed, Jul 02, 2008 at 10:38:51PM +0200, Johannes Hofmann wrote: > Hi, > > attached patch uses delete[] for deleting arrays as required by C++. Committed. Hey, a quick test showed memory usage reduction! -- Cheers Jorge.-

July 3, 2008

patch: (any Solarians in the audience?) rm nanosleep solaris workaround

by jcid＠dillo.org

On Tue, Jul 01, 2008 at 09:57:59PM +0000, corvid wrote: > It seems that there was a nanosleep() in src/IO/dpi.c long ago, > but I can't find any nanosleep()s anywhere anymore, so... Committed. Let's hope for somebody to have FLTK2 and dillo2 in Solaris! :) -- Cheers Jorge.-

July 3, 2008

iterator related bugs

by jcid＠dillo.org

On Wed, Jul 02, 2008 at 09:11:12PM +0200, Johannes Hofmann wrote: > Hi, > > while looking for the reason of the rare selection related crashes I > found something that seems like a severe bunch of bugs in the > iterator code. > It might be related to the crash, but I'm not 100% sure yet. > > There is this innocent comment in front of the DeepIterator > constructor in iterator.cc: > > If you want to continue using \em it, pass it->cloneIterator(). > > This means, that the DeepIterator constructor eats up the Iterator it > get's passed a pointer to! > > The evil thing is that every method that passes one of it's > arguments to the DeepIterator constructor inherits this property, > e.g: > > DeepIterator::createVariant() and as a consequence also: > SelectionState::switchLinkToSelection() and > SelectionState::adjustSelection() > > In Iterator::scrollTo() Iterators that are used after being > passed to the DeepIterator constructor. > > In SelectionState::buttonRelease() an iterator that has been passed > to switchLinkToSelection() might be used afterwards as argument to > adjustSelection(). > > I would propose to change the behaviour of the DeepIterator > constructor and always clone its input Iterator. > Better be safe than sorry and the fact that there is already several > bugs related to this shows, that the current behaviour - even if > it's documented - is really dangerous. > > What do you think? I agree with you. This is an API which already has hard-to-find side effects. Set DeepIterator to always clone, and if there's a performance penalty that's worth addressing, we may add a reference-parameter version of the constructor. -- Cheers Jorge.-

July 3, 2008

[patch] use delete[] where necessary

by Johannes.Hofmann＠gmx.de

Hi, attached patch uses delete[] for deleting arrays as required by C++. Cheers, Johannes

July 2, 2008

[patch] fix findtext for single char searches

by Johannes.Hofmann＠gmx.de

Hi, attached patch fixes a segfault when searching for single characters. Cheers, Johannes

July 2, 2008

iterator related bugs

by Johannes.Hofmann＠gmx.de

Hi, while looking for the reason of the rare selection related crashes I found something that seems like a severe bunch of bugs in the iterator code. It might be related to the crash, but I'm not 100% sure yet. There is this innocent comment in front of the DeepIterator constructor in iterator.cc: If you want to continue using \em it, pass it->cloneIterator(). This means, that the DeepIterator constructor eats up the Iterator it get's passed a pointer to! The evil thing is that every method that passes one of it's arguments to the DeepIterator constructor inherits this property, e.g: DeepIterator::createVariant() and as a consequence also: SelectionState::switchLinkToSelection() and SelectionState::adjustSelection() In Iterator::scrollTo() Iterators that are used after being passed to the DeepIterator constructor. In SelectionState::buttonRelease() an iterator that has been passed to switchLinkToSelection() might be used afterwards as argument to adjustSelection(). I would propose to change the behaviour of the DeepIterator constructor and always clone its input Iterator. Better be safe than sorry and the fact that there is already several bugs related to this shows, that the current behaviour - even if it's documented - is really dangerous. What do you think? Cheers, Johannes

July 2, 2008