February 2009 - Dillo-dev - lists.mailman3.com

Fun with autocnf and mercurial

by joerg.sonnenberger＠web.de

On Wed, Feb 18, 2009 at 10:49:36AM +0000, Jeremy Henty wrote: > I'm completely convinced that this really adds any safety, but it's > easy enough to do. Revised patch attached. Looks good. > BTW: the autoconf documentation frowns on this sort of thing, from > (autoconf.info)Limitations of Builtins: > > `test' (files) > To enable `configure' scripts to support cross-compilation, > they shouldn't do anything that tests features of the build > system instead of the host system. This only applies for checking things in the host filesystem. E.g. testing for the existance of /etc/passwd would be bad for cross-compilation. > --- a/src/dillo.cc > +++ b/src/dillo.cc > @@ -211,6 +211,11 @@ > int width = D_GEOMETRY_DEFAULT_WIDTH, height = D_GEOMETRY_DEFAULT_HEIGHT; > char **opt_argv; > > +#if HG > + printf("HG current: %s\n", HG_CURRENT); > + printf("HG qparent: %s\n", HG_QPARENT); > +#endif > + Any reason why this are two lines? Joerg

Feb. 18, 2009

Fun with autocnf and mercurial

by onepoint＠starurchin.org

On Wed, Feb 18, 2009 at 10:49:36AM +0000, Jeremy Henty wrote: > I'm completely convinced that this really adds any safety, D'oh: I meant to write "I'm *not* completely convinced...". Regards, Jeremy Henty

Feb. 18, 2009

Leaking image structures

by onepoint＠starurchin.org

On Tue, Feb 17, 2009 at 08:39:04PM +0100, Hofmann Johannes wrote: > I guess there is a leak when image loading is off. Not the leaks in my valgrind logs: I always have image loading on. Regards, Jeremy Henty

Feb. 18, 2009

Fun with autocnf and mercurial

by onepoint＠starurchin.org

On Tue, Feb 17, 2009 at 09:08:33PM +0100, Joerg Sonnenberger wrote: > On Tue, Feb 17, 2009 at 07:52:13PM +0000, Jeremy Henty wrote: > > Do you mean that we should worry that there might be an hg in > > $PATH that is not Mercurial? Is it really necessary to be that > > cautious? > > It would be nice, yes. What about just wrapping this into > if [ -d .hg ]; ... fi or something like that? I'm completely convinced that this really adds any safety, but it's easy enough to do. Revised patch attached. BTW: the autoconf documentation frowns on this sort of thing, from (autoconf.info)Limitations of Builtins: `test' (files) To enable `configure' scripts to support cross-compilation, they shouldn't do anything that tests features of the build system instead of the host system. So I guess if we want to be truly GNU-ly correct we should disable this code unless the builder specifies some enable-* configure flag. However, the code won't break any cross-compilation so it's not critical. Regards, Jeremy Henty

Feb. 18, 2009

Leaking image structures

by corvid＠lavabit.com

Johannes wrote: > On Wed, Feb 18, 2009 at 08:26:52AM +0000, corvid wrote: > > > On Wed, Feb 18, 2009 at 12:40:57AM +0000, corvid wrote: > > > > > Can you please try attached patch with valgrind and image loading on > > > > > and off. > > > > > > > > Is this a proposed patch for the tree or an experiment in leak-finding? > > > > > > For the tree, if it proofs to fix the leaks and doesn't cause > > > others. > > > > If we always add Image, the code that loads images manually will think > > it hasn't been loaded yet. > > > > Ah ok. I guess I misunderstood Html_add_new_linkimage() completely. > Please check the updated patch. > > Thanks, > Johannes > > PS: The point of the patch is to actually do reference counting: > Whenever we create a new reference (pointer) to an image, we call > a_Image_ref(), when such a pointer is deleted we call > a_Image_unref(). > If I see it correctly, the permanent pointers to Image > structures are in html->images and in the DilloWeb structure. It's bedtime now, so I haven't looked at this carefully yet, but you might need an unref in DilloHtml::loadImages() then.

Feb. 18, 2009

Leaking image structures

by Johannes.Hofmann＠gmx.de

On Wed, Feb 18, 2009 at 08:26:52AM +0000, corvid wrote: > > On Wed, Feb 18, 2009 at 12:40:57AM +0000, corvid wrote: > > > > Can you please try attached patch with valgrind and image loading on > > > > and off. > > > > > > Is this a proposed patch for the tree or an experiment in leak-finding? > > > > For the tree, if it proofs to fix the leaks and doesn't cause > > others. > > If we always add Image, the code that loads images manually will think > it hasn't been loaded yet. > Ah ok. I guess I misunderstood Html_add_new_linkimage() completely. Please check the updated patch. Thanks, Johannes PS: The point of the patch is to actually do reference counting: Whenever we create a new reference (pointer) to an image, we call a_Image_ref(), when such a pointer is deleted we call a_Image_unref(). If I see it correctly, the permanent pointers to Image structures are in html->images and in the DilloWeb structure.

Feb. 18, 2009

Fun with autocnf and mercurial

by joerg.sonnenberger＠web.de

On Tue, Feb 17, 2009 at 07:52:13PM +0000, Jeremy Henty wrote: > > I know that hg does the check internally, but that also depends on > > that no other hg binary can be found. > > Do you mean that we should worry that there might be an hg in $PATH > that is not Mercurial? Is it really necessary to be that cautious? It would be nice, yes. What about just wrapping this into if [ -d .hg ]; ... fi or something like that? Joerg

Feb. 17, 2009

Fun with autocnf and mercurial

by onepoint＠starurchin.org

On Tue, Feb 17, 2009 at 07:49:33PM +0100, Joerg Sonnenberger wrote: > Point is, release build should never differ whether hg is installed > or not. I absolutely agree, but if by "release build" you mean "a build not in a Mercurial repository" then that is already true of the current version of the patch. If you're not in a Mercurial repository then exactly the same code is compiled whether Mercurial is installed or not. > I know that hg does the check internally, but that also depends on > that no other hg binary can be found. Do you mean that we should worry that there might be an hg in $PATH that is not Mercurial? Is it really necessary to be that cautious? If we must handle host OSes that break our attempts to auto-detect hg then we will have to make the code conditional on a new configure flag (eg. --enable-hg-logging), with the proviso that if you supply the flag then the obligation is on you to ensure that your hg is a properly working Mercurial executable. That's easy to implement. I hope I don't seem obstinate, but I am finding it hard to see the scenario that supposedly breaks the current patch. (Unless the scenario is "hg might not be Mercurial" in which case I have already described what I think is the only sensible fix.) Regards, Jeremy Henty

Feb. 17, 2009

Leaking image structures

by Johannes.Hofmann＠gmx.de

On Wed, Feb 04, 2009 at 02:47:51PM +0000, Jeremy Henty wrote: > > One of the biggest leaks that valgrind reports is image structures. > I'm attaching logs that show memory being allocated by a_Png_new() and > cousins and not being freed. There are also frequent leaks of memory > allocated by the image libraries from within the various image > callbacks, but I suspect these are just another effect of the Dillo > image structures not being freed. > > Unfortunately I'm having a hard time tracking these leaks down. They > regularly appear after long browsing sessions, but I haven't found any > single page that reproduces them. I suspect they are caused by some > race condition inside the CCC rather than the HTML parser. I am > browsing with image loading enabled, and if I understand the code > correctly that means that the Web structures in the CCC own all the > references to image structures. > > The fact that image structures are reference counted makes it hard to > know when the structures should be freed anyway. Indeed, does Dillo > even ensure that image structures are freed? Maybe this is just a > symptom of incomplete cleanup at exit rather than a true leak? > > I've looked over the code and can't see any obvious leaks so I'm > rather stuck. Any tips for tracking this down? Even if it's not a > true leak it would be good to have a fix that cleaned up the valgrind > logs. I guess there is a leak when image loading is off. In that case images are not put in the html->images list and therefore not unrefed when leaving the page. Can you please try attached patch with valgrind and image loading on and off. Regards, Johannes

Feb. 17, 2009

Fun with autocnf and mercurial

by joerg.sonnenberger＠web.de

On Tue, Feb 17, 2009 at 05:54:17PM +0000, Jeremy Henty wrote: > On Tue, Feb 17, 2009 at 06:40:01PM +0100, Joerg Sonnenberger wrote: > > > Please ensure that this is not part of releases and/or if the .hg > > directory is missing. > > It already does this. If you are not in a Mercurial repository (or if > Mercurial is not installed) then the "if hg id ..." test fails and HG > is #defined to 0 instead of 1. The extra logging code is wrapped in a > "#if HG" so it will be removed by the preprocessor. I just tested > this by temporarily renaming .hg and rebuilding and it worked as > intended. Point is, release build should never differ whether hg is installed or not. I know that hg does the check internally, but that also depends on that no other hg binary can be found. Joerg

Feb. 17, 2009