57822 2011-04-05 02:08:13 -0700 Build fix for YarrParser.h 2011-04-05 15:04:59 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 kbalazs webkit-unassigned abecsi barraclough christoph msaboff ossy pvarga oldest_to_newest 379383 0 kbalazs 2011-04-05 02:08:13 -0700 It does not compile for a long time in my local environment which is: gcc (SUSE Linux) 4.5.0 20100604 [gcc-4_5-branch revision 160292]. 379384 1 88191 kbalazs 2011-04-05 02:10:52 -0700 Created attachment 88191 Patch 379395 2 ossy 2011-04-05 02:21:16 -0700 I'm not sure which initialization value should we use, ask the Yarr experts, Cpt Stampho and/or Gavin. *** This bug has been marked as a duplicate of bug 55576 *** 379407 3 kbalazs 2011-04-05 02:44:27 -0700 The other bug has not patch and I won't re-upload mine. 0 is seems like a natural value for UChar. Here is some grep output: Source/JavaScriptCore/runtime/StringPrototype.cpp:865: UChar ored = 0; Source/JavaScriptCore/runtime/StringPrototype.cpp:905: UChar ored = 0; Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:81: UChar u = 0; 379414 4 ossy 2011-04-05 03:12:06 -0700 (In reply to comment #3) > The other bug has not patch and I won't re-upload mine. Nobody asked you to upload your patch again ... > 0 is seems like a natural value for UChar. Here is some grep output: > Source/JavaScriptCore/runtime/StringPrototype.cpp:865: UChar ored = 0; > Source/JavaScriptCore/runtime/StringPrototype.cpp:905: UChar ored = 0; > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:81: UChar u = 0; It is absolutely useless information. We can't fix the bug without understanding the Yarr parser. I don't know if m_character is used somewhere before initializing. But using a possibly wrong init value is so wrong as using an uninitialized value. Gavin, could you check if this initialization is correct or not? 379416 5 ossy 2011-04-05 03:13:37 -0700 *** Bug 55576 has been marked as a duplicate of this bug. *** 379442 6 kbalazs 2011-04-05 04:28:53 -0700 (In reply to comment #4) > (In reply to comment #3) > > The other bug has not patch and I won't re-upload mine. > Nobody asked you to upload your patch again ... > > > 0 is seems like a natural value for UChar. Here is some grep output: > > Source/JavaScriptCore/runtime/StringPrototype.cpp:865: UChar ored = 0; > > Source/JavaScriptCore/runtime/StringPrototype.cpp:905: UChar ored = 0; > > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:81: UChar u = 0; > > It is absolutely useless information. We can't fix the bug without understanding the Yarr parser. I don't know if m_character is used > somewhere before initializing. Don't you think that 0 is better than memory garbage? What else can be the initialization value for an integral type? I don't understand your concerns. 379653 7 88191 darin 2011-04-05 09:40:15 -0700 Comment on attachment 88191 Patch I think that we don’t want to initialize things just on general principles or because “initialized is better than uninitialized”. It’s best to set the initial value based on an understanding of how the value will be used. 379658 8 88191 darin 2011-04-05 09:43:01 -0700 Comment on attachment 88191 Patch If we do make this change, it will be just to make the compiler happy. When m_state is Empty it is illegal to look at the character value, so initializing it is only helpful to mitigate programming mistakes. If m_state is set to another state where m_character is used, then m_character will be set appropriately. The compiler can’t analyze the data flow well enough to see the relationship so it thinks there is a potential problem here, but there is none. 379880 9 kbalazs 2011-04-05 14:59:54 -0700 (In reply to comment #8) > (From update of attachment 88191 [details]) > If we do make this change, it will be just to make the compiler happy. When m_state is Empty it is illegal to look at the character value, so initializing it is only helpful to mitigate programming mistakes. If m_state is set to another state where m_character is used, then m_character will be set appropriately. > > The compiler can’t analyze the data flow well enough to see the relationship so it thinks there is a potential problem here, but there is none. Sure. The only motivation behind this patch is to make the compiler happy. Thanks for the review! 379888 10 88191 kbalazs 2011-04-05 15:04:52 -0700 Comment on attachment 88191 Patch Clearing flags on attachment: 88191 Committed r82988: <http://trac.webkit.org/changeset/82988> 379889 11 kbalazs 2011-04-05 15:04:59 -0700 All reviewed patches have been landed. Closing bug. 88191 2011-04-05 02:10:52 -0700 2011-04-05 15:04:52 -0700 Patch bug-57822-20110405111050.patch text/plain 1141 kbalazs U3VidmVyc2lvbiBSZXZpc2lvbjogODI3NzEKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDRl YWM0NDYxMDk5NWEyZGY1NzAzMGM3YmI5NjYwYTg0Y2JiMDhkNzQuLjAyODIyNGM1MDE3NmZkYzhi YzcwOTAxMzZlNDdiZDg2MjMxZDczZjcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDEzIEBACisyMDExLTA0LTA1ICBCYWxhenMgS2VsZW1lbiAgPGtiYWxhenNAd2Via2l0Lm9y Zz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBCdWls ZCBmaXggZm9yIFlhcnJQYXJzZXIuaAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9NTc4MjIKKworICAgICAgICAqIHlhcnIvWWFyclBhcnNlci5oOgorICAg ICAgICAoSlNDOjpZYXJyOjpQYXJzZXI6OkNoYXJhY3RlckNsYXNzUGFyc2VyRGVsZWdhdGU6OkNo YXJhY3RlckNsYXNzUGFyc2VyRGVsZWdhdGUpOgorCiAyMDExLTA0LTAxICBHZW9mZnJleSBHYXJl biAgPGdnYXJlbkBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgT2xpdmVyIEh1bnQu CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUveWFyci9ZYXJyUGFyc2VyLmggYi9T b3VyY2UvSmF2YVNjcmlwdENvcmUveWFyci9ZYXJyUGFyc2VyLmgKaW5kZXggYzU5MzEwYTdjYzQ3 NmExOWUzZWQ4NDQ3YzA4NmE3ODA4NTIwMmNiNy4uNjVmYjQxZGExMjUyMjFkYmFjMmQ5YWZhZjZh OGFmYTdhOGZjMTkyMyAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3lhcnIvWWFy clBhcnNlci5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS95YXJyL1lhcnJQYXJzZXIuaApA QCAtNzgsNiArNzgsNyBAQCBwcml2YXRlOgogICAgICAgICAgICAgOiBtX2RlbGVnYXRlKGRlbGVn YXRlKQogICAgICAgICAgICAgLCBtX2VycihlcnIpCiAgICAgICAgICAgICAsIG1fc3RhdGUoRW1w dHkpCisgICAgICAgICAgICAsIG1fY2hhcmFjdGVyKDApCiAgICAgICAgIHsKICAgICAgICAgfQog Cg==