<ol>'s leak the labels of their items
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); in Html_tag_open_li (in src/html.cc). The dStrdup()ed memory is never freed. I think that the problem is that the TextBlock destructor only deletes the content of a word if the content type is WIDGET or ANCHOR, whereas it should also handle the case when the content type is TEXT. Perhaps initWithText() should copy the string argument itself with new[] so that ~TextBlock could safely delete[] it? The only two references to initWithText() are in html.cc and one of the widget tests, so this would be an easy change. Regards, Jeremy Henty
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. Cheers, Johannes
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
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
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
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
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
On Wed, Jan 28, 2009 at 01:06:51PM +0000, Jeremy Henty wrote:
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.
Went with the former for now, but we will need to switch to the latter to support "inherit" or properties that can hold strings _or_ enums. Please check current tip. Cheers, Johannes
On Wed, Jan 28, 2009 at 10:44:11PM +0100, Hofmann Johannes wrote:
On Wed, Jan 28, 2009 at 01:06:51PM +0000, Jeremy Henty wrote:
Went with the former for now, but we will need to switch to the latter to support "inherit" or properties that can hold strings _or_ enums. Please check current tip.
I still see a leak. I'm attaching a test case, the valgrind log and a suppression that stops the report. Regards, Jeremy Henty
On Thu, Jan 29, 2009 at 11:43:36AM +0000, Jeremy Henty wrote:
On Wed, Jan 28, 2009 at 10:44:11PM +0100, Hofmann Johannes wrote:
On Wed, Jan 28, 2009 at 01:06:51PM +0000, Jeremy Henty wrote:
Went with the former for now, but we will need to switch to the latter to support "inherit" or properties that can hold strings _or_ enums. Please check current tip.
I still see a leak. I'm attaching a test case, the valgrind log and a suppression that stops the report.
Not sure about those. Maybe it's reporting the strings allocated for the user-agent style? Those are allocated once and never freed. Can you please retry with the user-agent style parsing disabled: diff -r 4b5125e3ca89 src/css.cc --- a/src/css.cc Thu Jan 29 12:38:20 2009 +0100 +++ b/src/css.cc Thu Jan 29 13:24:16 2009 +0100 @@ -327,7 +327,7 @@ CssContext::CssContext () { sheet[CSS_PRIMARY_USER] = userStyle; sheet[CSS_PRIMARY_USER_IMPORTANT] = userImportantStyle; - buildUserAgentStyle (); + //buildUserAgentStyle (); buildUserStyle (); } Cheers, Johannes
On Thu, Jan 29, 2009 at 01:25:29PM +0100, Hofmann Johannes wrote:
Can you please retry with the user-agent style parsing disabled:
diff -r 4b5125e3ca89 src/css.cc --- a/src/css.cc Thu Jan 29 12:38:20 2009 +0100 +++ b/src/css.cc Thu Jan 29 13:24:16 2009 +0100 @@ -327,7 +327,7 @@ CssContext::CssContext () { sheet[CSS_PRIMARY_USER] = userStyle; sheet[CSS_PRIMARY_USER_IMPORTANT] = userImportantStyle;
- buildUserAgentStyle (); + //buildUserAgentStyle (); buildUserStyle (); }
That takes out most of the "indirectly lost" memory but doesn't affect the "definitely lost" stuff that valgrind generates the suppression for. Log attached. Regards, Jeremy Henty
On Thu, Jan 29, 2009 at 01:03:29PM +0000, Jeremy Henty wrote:
On Thu, Jan 29, 2009 at 01:25:29PM +0100, Hofmann Johannes wrote:
Can you please retry with the user-agent style parsing disabled:
diff -r 4b5125e3ca89 src/css.cc --- a/src/css.cc Thu Jan 29 12:38:20 2009 +0100 +++ b/src/css.cc Thu Jan 29 13:24:16 2009 +0100 @@ -327,7 +327,7 @@ CssContext::CssContext () { sheet[CSS_PRIMARY_USER] = userStyle; sheet[CSS_PRIMARY_USER_IMPORTANT] = userImportantStyle;
- buildUserAgentStyle (); + //buildUserAgentStyle (); buildUserStyle (); }
That takes out most of the "indirectly lost" memory but doesn't affect the "definitely lost" stuff that valgrind generates the suppression for. Log attached.
Next try. Please check attached patch. Thanks for testing this, Johannes
On Thu, Jan 29, 2009 at 02:48:54PM +0000, Jeremy Henty wrote:
On Thu, Jan 29, 2009 at 02:54:23PM +0100, Hofmann Johannes wrote:
Next try. Please check attached patch.
No difference. :-(
Does this also happen if you just start dillo with the splash screen and quit again (without accessing any websites)? Or which websites do you access to reproduce this? Johannes
On Thu, Jan 29, 2009 at 03:52:45PM +0100, Hofmann Johannes wrote:
On Thu, Jan 29, 2009 at 02:48:54PM +0000, Jeremy Henty wrote:
On Thu, Jan 29, 2009 at 02:54:23PM +0100, Hofmann Johannes wrote:
Next try. Please check attached patch.
No difference. :-(
Does this also happen if you just start dillo with the splash screen and quit again (without accessing any websites)? Or which websites do you access to reproduce this?
Never mind. I think I've got it. See attached patch. Cheers, Johannes
On Thu, Jan 29, 2009 at 04:08:12PM +0100, Hofmann Johannes wrote:
Never mind. I think I've got it. See attached patch.
That did it! That is, it removed the "definitely lost" errors. The "indirectly lost errors" are still there. I'm still not sure how to interpret them. They could be artifacts of the other leaks that I am currently suppressing. Attached the log and the test HTML I was using. Jeremy Henty
On Wed, Jan 28, 2009 at 10:44:11PM +0100, Hofmann Johannes wrote:
On Wed, Jan 28, 2009 at 01:06:51PM +0000, Jeremy Henty wrote:
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.
Went with the former for now, but we will need to switch to the latter to support "inherit" or properties that can hold strings _or_ enums. Please check current tip.
These patches produce lots of warnings in the test directory: make[2]: Entering directory `/home/jcid/C/Dillo/d2/dillo-wc/test' dw_anchors_test.cc: In function 'int main(int, char**)': dw_anchors_test.cc:120: warning: deprecated conversion from string constant to 'char*' dw_example.cc: In function 'int main(int, char**)': dw_example.cc:49: warning: deprecated conversion from string constant to 'char*' dw_find_test.cc: In function 'int main(int, char**)': dw_find_test.cc:93: warning: deprecated conversion from string constant to 'char*' dw_links.cc: In function 'int main(int, char**)': dw_links.cc:88: warning: deprecated conversion from string constant to 'char*' dw_links2.cc: In function 'int main(int, char**)': dw_links2.cc:116: warning: deprecated conversion from string constant to 'char*' dw_images_simple.cc: In function 'int main(int, char**)': dw_images_simple.cc:110: warning: deprecated conversion from string constant to 'char*' dw_images_scaled.cc: In function 'int main(int, char**)': dw_images_scaled.cc:112: warning: deprecated conversion from string constant to 'char*' dw_images_scaled2.cc: In function 'int main(int, char**)': dw_images_scaled2.cc:87: warning: deprecated conversion from string constant to 'char*' dw_lists.cc: In function 'int main(int, char**)': dw_lists.cc:53: warning: deprecated conversion from string constant to 'char*' dw_table_aligned.cc: In function 'int main(int, char**)': dw_table_aligned.cc:56: warning: deprecated conversion from string constant to 'char*' dw_table.cc: In function 'int main(int, char**)': dw_table.cc:61: warning: deprecated conversion from string constant to 'char*' dw_border_test.cc: In function 'int main(int, char**)': dw_border_test.cc:57: warning: deprecated conversion from string constant to 'char*' dw_resource_test.cc: In function 'int main(int, char**)': dw_resource_test.cc:54: warning: deprecated conversion from string constant to 'char*' dw_ui_test.cc: In function 'int main(int, char**)': dw_ui_test.cc:60: warning: deprecated conversion from string constant to 'char*' make[2]: Leaving directory `/home/jcid/C/Dillo/d2/dillo-wc/test' when compiled with: CFLAGS="-g -O0 -W" CXXFLAGS="-g -O0 -W" ./configure --enable-ssl Please update test/ too. -- Cheers Jorge.- ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________
On Thu, Jan 29, 2009 at 09:37:31AM -0300, Jorge Arellano Cid wrote:
On Wed, Jan 28, 2009 at 10:44:11PM +0100, Hofmann Johannes wrote:
On Wed, Jan 28, 2009 at 01:06:51PM +0000, Jeremy Henty wrote:
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.
Went with the former for now, but we will need to switch to the latter to support "inherit" or properties that can hold strings _or_ enums. Please check current tip.
These patches produce lots of warnings in the test directory:
I can't reproduce the warnings, but I think I know the reason. It's changeset 9a7684395cca. Can you please check attached patch which tries to fix the underlying issue in an other way. Comments are welcome! We could also backout 9a7684395cca and leave things as they were before, but deleting memory allocated with strdup() is a bug. Cheers, Johannes
make[2]: Entering directory `/home/jcid/C/Dillo/d2/dillo-wc/test' dw_anchors_test.cc: In function 'int main(int, char**)': dw_anchors_test.cc:120: warning: deprecated conversion from string constant to 'char*' dw_example.cc: In function 'int main(int, char**)': dw_example.cc:49: warning: deprecated conversion from string constant to 'char*' dw_find_test.cc: In function 'int main(int, char**)': dw_find_test.cc:93: warning: deprecated conversion from string constant to 'char*' dw_links.cc: In function 'int main(int, char**)': dw_links.cc:88: warning: deprecated conversion from string constant to 'char*' dw_links2.cc: In function 'int main(int, char**)': dw_links2.cc:116: warning: deprecated conversion from string constant to 'char*' dw_images_simple.cc: In function 'int main(int, char**)': dw_images_simple.cc:110: warning: deprecated conversion from string constant to 'char*' dw_images_scaled.cc: In function 'int main(int, char**)': dw_images_scaled.cc:112: warning: deprecated conversion from string constant to 'char*' dw_images_scaled2.cc: In function 'int main(int, char**)': dw_images_scaled2.cc:87: warning: deprecated conversion from string constant to 'char*' dw_lists.cc: In function 'int main(int, char**)': dw_lists.cc:53: warning: deprecated conversion from string constant to 'char*' dw_table_aligned.cc: In function 'int main(int, char**)': dw_table_aligned.cc:56: warning: deprecated conversion from string constant to 'char*' dw_table.cc: In function 'int main(int, char**)': dw_table.cc:61: warning: deprecated conversion from string constant to 'char*' dw_border_test.cc: In function 'int main(int, char**)': dw_border_test.cc:57: warning: deprecated conversion from string constant to 'char*' dw_resource_test.cc: In function 'int main(int, char**)': dw_resource_test.cc:54: warning: deprecated conversion from string constant to 'char*' dw_ui_test.cc: In function 'int main(int, char**)': dw_ui_test.cc:60: warning: deprecated conversion from string constant to 'char*' make[2]: Leaving directory `/home/jcid/C/Dillo/d2/dillo-wc/test'
when compiled with: CFLAGS="-g -O0 -W" CXXFLAGS="-g -O0 -W" ./configure --enable-ssl
Please update test/ too.
-- Cheers Jorge.-
______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________
_______________________________________________ Dillo-dev mailing list Dillo-dev@dillo.org http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev
On Thu, Jan 29, 2009 at 02:33:30PM +0100, Hofmann Johannes wrote:
On Thu, Jan 29, 2009 at 09:37:31AM -0300, Jorge Arellano Cid wrote:
On Wed, Jan 28, 2009 at 10:44:11PM +0100, Hofmann Johannes wrote:
On Wed, Jan 28, 2009 at 01:06:51PM +0000, Jeremy Henty wrote:
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.
Went with the former for now, but we will need to switch to the latter to support "inherit" or properties that can hold strings _or_ enums. Please check current tip.
These patches produce lots of warnings in the test directory:
I can't reproduce the warnings,
I'm using: gcc version 4.3.2 CFLAGS="-g -O0 -W" CXXFLAGS="-g -O0 -W" ./configure --enable-ssl
but I think I know the reason. It's changeset 9a7684395cca. Can you please check attached patch which tries to fix the underlying issue in an other way. Comments are welcome! We could also backout 9a7684395cca and leave things as they were before, but deleting memory allocated with strdup() is a bug.
Yes, that's a bug. I think the solution is much simpler. The attached patch, applied on current tip, solves the issue. Please try it and commit if you like it. ;) -- Cheers Jorge.- ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________
On Thu, Jan 29, 2009 at 11:38:34AM -0300, Jorge Arellano Cid wrote:
On Thu, Jan 29, 2009 at 02:33:30PM +0100, Hofmann Johannes wrote:
On Thu, Jan 29, 2009 at 09:37:31AM -0300, Jorge Arellano Cid wrote:
On Wed, Jan 28, 2009 at 10:44:11PM +0100, Hofmann Johannes wrote:
On Wed, Jan 28, 2009 at 01:06:51PM +0000, Jeremy Henty wrote:
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.
Went with the former for now, but we will need to switch to the latter to support "inherit" or properties that can hold strings _or_ enums. Please check current tip.
These patches produce lots of warnings in the test directory:
I can't reproduce the warnings,
I'm using: gcc version 4.3.2 CFLAGS="-g -O0 -W" CXXFLAGS="-g -O0 -W" ./configure --enable-ssl
but I think I know the reason. It's changeset 9a7684395cca. Can you please check attached patch which tries to fix the underlying issue in an other way. Comments are welcome! We could also backout 9a7684395cca and leave things as they were before, but deleting memory allocated with strdup() is a bug.
Yes, that's a bug.
I think the solution is much simpler. The attached patch, applied on current tip, solves the issue. Please try it and commit if you like it. ;)
Right. It's really not worth the effort to avoid this cast. Committed! Thanks, Johannes
On Thu, Jan 29, 2009 at 03:40:28PM +0100, Hofmann Johannes wrote:
On Thu, Jan 29, 2009 at 11:38:34AM -0300, Jorge Arellano Cid wrote:
On Thu, Jan 29, 2009 at 02:33:30PM +0100, Hofmann Johannes wrote:
On Thu, Jan 29, 2009 at 09:37:31AM -0300, Jorge Arellano Cid wrote:
On Wed, Jan 28, 2009 at 10:44:11PM +0100, Hofmann Johannes wrote:
On Wed, Jan 28, 2009 at 01:06:51PM +0000, Jeremy Henty wrote:
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.
Went with the former for now, but we will need to switch to the latter to support "inherit" or properties that can hold strings _or_ enums. Please check current tip.
These patches produce lots of warnings in the test directory:
I can't reproduce the warnings,
I'm using: gcc version 4.3.2 CFLAGS="-g -O0 -W" CXXFLAGS="-g -O0 -W" ./configure --enable-ssl
but I think I know the reason. It's changeset 9a7684395cca. Can you please check attached patch which tries to fix the underlying issue in an other way. Comments are welcome! We could also backout 9a7684395cca and leave things as they were before, but deleting memory allocated with strdup() is a bug.
Yes, that's a bug.
I think the solution is much simpler. The attached patch, applied on current tip, solves the issue. Please try it and commit if you like it. ;)
Right. It's really not worth the effort to avoid this cast.
Committed!
Good. BTW, when receiving spare patches, after applying them and testing, this command comes very handy to commit: hg ci -u <User Info> e.g. hg ci -u "corvid <corvid@lavabit.com>" -- Cheers Jorge.- ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________
participants (3)
-
jcid＠dillo.org -
Johannes.Hofmann＠gmx.de -
onepoint＠starurchin.org