June 2008 - Dillo-dev - lists.mailman3.com

patch: view gzipped local files

by corvid＠lavabit.com

I'd been considering this for a while, but couldn't decide whether it had any real value. Then it occurred to me that if people are often using dillo as a local document viewer for minimalist systems, they may value the ability to compress those documents.

June 14, 2008

form.cc: more refactoring

by onepoint＠starurchin.org

A set of small patches that clean up form.cc , mostly to do with DilloHtmlSelect . * refactor by creating new functions: - a_Html_parse_stash() - DilloHtmlSelect::ensureSelection() - DilloHtmlSelect::addOptionsTo() - DilloHtmlSelect::appendValuesTo() - DilloHtmlSelect::getCurrentOption() (cf. other getCurrent* methods) * make DilloHtmlSelect::options private * rename DilloHtmlInput::getInputValues() to appendValuesTo() (the new name is more mnemonic and consistent with the DilloHtmlSelect method above) Regards, Jeremy Henty

June 14, 2008

Valgrind: hashing StyleAttr objects accesses uninitialized data

by onepoint＠starurchin.org

On Thu, Jun 12, 2008 at 09:56:49PM +0200, Johannes Hofmann wrote: > On Wed, Jun 11, 2008 at 11:36:29PM +0100, Jeremy Henty wrote: > > BTW, StyleAttrs::initValues() fails to set some other data members > > such as font and color , but fixing this has no effect on the > > valgrind warnings. > > As far as I can see this is intentional. font and color have to be > set explicitely by the caller of Style::create(). And if valgrind > does not complain, that seems to be the case. OK, then it's not a problem. Thanks for checking it out. Regards, Jeremy henty

June 13, 2008

Valgrind: hashing StyleAttr objects accesses uninitialized data

by jcid＠dillo.org

On Thu, Jun 12, 2008 at 09:56:49PM +0200, Johannes Hofmann wrote: > yOn Wed, Jun 11, 2008 at 11:36:29PM +0100, Jeremy Henty wrote: > > On Wed, Jun 11, 2008 at 09:51:17PM +0200, Johannes Hofmann wrote: > > > > > I think even hashValue() should not access uninitialized data. Not > > > only because of the annoying valgrind messages. > > > > Got it! It's StyleAttrs::textAlignChar ! If I add "textAlignChar = > > '.';" to StyleAttrs::initValues() (which StyleAttrs::resetValues() > > already does!) then the valgrind warnings disappear. Only > > a_Html_tag_set_align_attr() sets textAlignChar , but it only does so > > if it sees "align='char'". > > Great. So it's just this inititialization missing. Patchlet > attached. Committed. -- Cheers Jorge.-

June 12, 2008

Valgrind: hashing StyleAttr objects accesses uninitialized data

by Johannes.Hofmann＠gmx.de

yOn Wed, Jun 11, 2008 at 11:36:29PM +0100, Jeremy Henty wrote: > On Wed, Jun 11, 2008 at 09:51:17PM +0200, Johannes Hofmann wrote: > > > I think even hashValue() should not access uninitialized data. Not > > only because of the annoying valgrind messages. > > Got it! It's StyleAttrs::textAlignChar ! If I add "textAlignChar = > '.';" to StyleAttrs::initValues() (which StyleAttrs::resetValues() > already does!) then the valgrind warnings disappear. Only > a_Html_tag_set_align_attr() sets textAlignChar , but it only does so > if it sees "align='char'". Great. So it's just this inititialization missing. Patchlet attached. > > BTW, StyleAttrs::initValues() fails to set some other data members > such as font and color , but fixing this has no effect on the valgrind > warnings. As far as I can see this is intentional. font and color have to be set explicitely by the caller of Style::create(). And if valgrind does not complain, that seems to be the case. Cheers, Johannes

June 12, 2008

Valgrind: hashing StyleAttr objects accesses uninitialized data

by onepoint＠starurchin.org

On Wed, Jun 11, 2008 at 09:51:17PM +0200, Johannes Hofmann wrote: > I will try to fix that. Can you please post some of the errors > reported by valgrind? I've attached four log snippets showing valgrind warnings from: * hashTable::get() * hashTable::put() * hashTable::remove() * StyleAttrs::equals() Regards, Jeremy Henty

June 12, 2008

Valgrind: hashing StyleAttr objects accesses uninitialized data

by onepoint＠starurchin.org

On Wed, Jun 11, 2008 at 09:51:17PM +0200, Johannes Hofmann wrote: > I think even hashValue() should not access uninitialized data. Not > only because of the annoying valgrind messages. Got it! It's StyleAttrs::textAlignChar ! If I add "textAlignChar = '.';" to StyleAttrs::initValues() (which StyleAttrs::resetValues() already does!) then the valgrind warnings disappear. Only a_Html_tag_set_align_attr() sets textAlignChar , but it only does so if it sees "align='char'". BTW, StyleAttrs::initValues() fails to set some other data members such as font and color , but fixing this has no effect on the valgrind warnings. Hope this helps, Jeremy Henty

June 12, 2008

Valgrind: hashing StyleAttr objects accesses uninitialized data

by Johannes.Hofmann＠gmx.de

On Wed, Jun 11, 2008 at 07:41:02PM +0100, Jeremy Henty wrote: > > Ever since StyleAttr objects were cached in a hash to promote sharing, > valgrind has reported nearly 1000 branches dependent on uninitialised > data when dillo renders the splash page. These all come from > StyleAttrs::hashValue() and StyleAttrs::equals() : if I hack > hashValue() to return ((intptr_t) this) and equals() to return (this > == otherAttrs) then the warnings disappear. > > Is this a problem? If these methods are accessing uninitialised data > members then what happens when that data is initialised? The return > values of hashValue() and equals() could change, which sounds like a > potential source of bugs. Even if it's not a bug, if the value of > hashValue() is unpredictable then we might not be sharing StyleAttr > objects as much as we could and so we could be wasting memory. > > If it is correct to have code that raises these warnings, perhaps > there could be a "#define VALGRIND" switch so developers could easily > compile a dillo that doesn't raise them? (It's possible to silence > them with a valgrind suppression file but that is tedious and risks > hiding other warnings. It would be much more straightforward to have > a valgrind-clean version of the code.) > > Comments are welcome, I think even hashValue() should not access uninitialized data. Not only because of the annoying valgrind messages. I will try to fix that. Can you please post some of the errors reported by valgrind? Johannes

June 11, 2008

Valgrind: hashing StyleAttr objects accesses uninitialized data

by onepoint＠starurchin.org

Ever since StyleAttr objects were cached in a hash to promote sharing, valgrind has reported nearly 1000 branches dependent on uninitialised data when dillo renders the splash page. These all come from StyleAttrs::hashValue() and StyleAttrs::equals() : if I hack hashValue() to return ((intptr_t) this) and equals() to return (this == otherAttrs) then the warnings disappear. Is this a problem? If these methods are accessing uninitialised data members then what happens when that data is initialised? The return values of hashValue() and equals() could change, which sounds like a potential source of bugs. Even if it's not a bug, if the value of hashValue() is unpredictable then we might not be sharing StyleAttr objects as much as we could and so we could be wasting memory. If it is correct to have code that raises these warnings, perhaps there could be a "#define VALGRIND" switch so developers could easily compile a dillo that doesn't raise them? (It's possible to silence them with a valgrind suppression file but that is tedious and risks hiding other warnings. It would be much more straightforward to have a valgrind-clean version of the code.) Comments are welcome, Jeremy Henty

June 11, 2008

Splitting html.cc: what next?

by onepoint＠starurchin.org

On Wed, Jun 11, 2008 at 12:13:13PM -0400, Jorge Arellano Cid wrote: > I'd say go with table... OK, here's the patch. It's a much smaller change than the form.cc patch (table.cc is only 10k) because table parsing is much simpler and form.cc includes form submission code as well as parsing. It doesn't make nearly as much of an impact on the size of html.cc . It also moves more declarations into html_common.hh (stuff to parse colours and lengths and set styles). As before developers will have to " make maintainer-clean ; ./autogen.sh " so that table.cc gets built. > ...and that's it. :-( I was looking forward to more slicing and dicing! ;-) What do others think? Regards, Jeremy Henty

June 11, 2008