62222 2011-06-07 11:29:22 -0700 JSStaticFunction and JSStaticValue are overly "const" 2011-06-07 19:00:48 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Minor --- 0 j.mont webkit-unassigned ggaren webkit.review.bot oldest_to_newest 416664 0 j.mont 2011-06-07 11:29:22 -0700 10/3/06 2:30 PM by John Harper: * SUMMARY JSStaticFunction has this definition: typedef struct { const char* const name; // FIXME: convert UTF8 JSObjectCallAsFunctionCallback callAsFunction; JSPropertyAttributes attributes; } JSStaticFunction; which means that it's impossible to initialize the name field dynamically. E.g. this code: JSStaticFunction *fun; fun->name = "something"; fails with a compiler error. There is no reason for the second of those two "const" qualifiers? A cast will be needed to set the structure field.. [...] it definitely gives a warning for me, in test.c: #include <JavaScriptCore/JavaScriptCore.h> static void foo (void) { JSStaticFunction *fun; fun->name = "something"; } then: $ cc -c test.c test.c: In function 'foo': test.c:7: error: assignment of read-only member 'name' in my actual code I did this: *(void **)&funptr->name = (void *) m->desc->jsName; 416671 1 96270 j.mont 2011-06-07 11:38:03 -0700 Created attachment 96270 proposed patch 416678 2 96270 darin 2011-06-07 11:41:34 -0700 Comment on attachment 96270 proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=96270&action=review > Source/JavaScriptCore/ChangeLog:5 > + Make JSStaticFunction and JSStaticValue less "const" Why? 416679 3 96270 darin 2011-06-07 11:42:33 -0700 Comment on attachment 96270 proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=96270&action=review Seems OK to change this. The other data members aren’t const, and there’s no strong argument for making these const. >> Source/JavaScriptCore/ChangeLog:5 >> + Make JSStaticFunction and JSStaticValue less "const" > > Why? I think I see why now. 416810 4 96270 ggaren 2011-06-07 14:56:52 -0700 Comment on attachment 96270 proposed patch Marking cq+ to get this landed by the commit queue. 416811 5 96270 ggaren 2011-06-07 14:59:58 -0700 Comment on attachment 96270 proposed patch Oops! The Windows EWS bot failed to compile, and showing this warning, which illustrates a real bug in your code: 7>c:\cygwin\home\buildbot\webkit\source\javascriptcore\api\tests\testapi.c(943) : error C2220: warning treated as error - no 'object' file generated 7>c:\cygwin\home\buildbot\webkit\source\javascriptcore\api\tests\testapi.c(943) : warning C4700: uninitialized local variable 'fun' used 7>c:\cygwin\home\buildbot\webkit\source\javascriptcore\api\tests\testapi.c(945) : warning C4700: uninitialized local variable 'val' used You can't just declare "fun" as a pointer -- you need to point to something. I'd suggest changing this to declare fun and val as variables on the stack. 416830 6 96309 j.mont 2011-06-07 15:20:37 -0700 Created attachment 96309 proposed patch sorry, didn't catch that in the original report. all fixed now :) 417008 7 96309 webkit.review.bot 2011-06-07 19:00:44 -0700 Comment on attachment 96309 proposed patch Clearing flags on attachment: 96309 Committed r88313: <http://trac.webkit.org/changeset/88313> 417009 8 webkit.review.bot 2011-06-07 19:00:48 -0700 All reviewed patches have been landed. Closing bug. 96270 2011-06-07 11:38:03 -0700 2011-06-07 15:20:37 -0700 proposed patch patch.txt text/plain 2481 j.mont SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gODgyNDgpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAK KzIwMTEtMDYtMDcgIEp1YW4gQy4gTW9udGVtYXlvciAgPGptb250QGFwcGxlLmNvbT4KKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNYWtlIEpTU3RhdGlj RnVuY3Rpb24gYW5kIEpTU3RhdGljVmFsdWUgbGVzcyAiY29uc3QiCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MjIyMgorCisgICAgICAgICogQVBJL0pT T2JqZWN0UmVmLmg6CisgICAgICAgICogQVBJL3Rlc3RzL3Rlc3RhcGkuYzoKKyAgICAgICAgKGNo ZWNrQ29uc3RuZXNzSW5KU09iamVjdE5hbWVzKToKKyAgICAgICAgKG1haW4pOgorICAgICAgICAq IEphdmFTY3JpcHRDb3JlLnhjb2RlcHJvai9wcm9qZWN0LnBieHByb2o6CisKIDIwMTEtMDYtMDYg IEphbWVzIFNpbW9uc2VuICA8c2ltb25qYW1AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmll d2VkIGJ5IEphbWVzIFJvYmluc29uLgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL0FQSS9K U09iamVjdFJlZi5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9BUEkvSlNP YmplY3RSZWYuaAkocmV2aXNpb24gODgxOTcpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJ L0pTT2JqZWN0UmVmLmgJKHdvcmtpbmcgY29weSkKQEAgLTI4OCw3ICsyODgsNyBAQCB0eXBlZGVm IEpTVmFsdWVSZWYKIEBmaWVsZCBhdHRyaWJ1dGVzIEEgbG9naWNhbGx5IE9SZWQgc2V0IG9mIEpT UHJvcGVydHlBdHRyaWJ1dGVzIHRvIGdpdmUgdG8gdGhlIHByb3BlcnR5LgogKi8KIHR5cGVkZWYg c3RydWN0IHsKLSAgICBjb25zdCBjaGFyKiBjb25zdCBuYW1lOworICAgIGNvbnN0IGNoYXIqIG5h bWU7CiAgICAgSlNPYmplY3RHZXRQcm9wZXJ0eUNhbGxiYWNrIGdldFByb3BlcnR5OwogICAgIEpT T2JqZWN0U2V0UHJvcGVydHlDYWxsYmFjayBzZXRQcm9wZXJ0eTsKICAgICBKU1Byb3BlcnR5QXR0 cmlidXRlcyBhdHRyaWJ1dGVzOwpAQCAtMzAyLDcgKzMwMiw3IEBAIHR5cGVkZWYgc3RydWN0IHsK IEBmaWVsZCBhdHRyaWJ1dGVzIEEgbG9naWNhbGx5IE9SZWQgc2V0IG9mIEpTUHJvcGVydHlBdHRy aWJ1dGVzIHRvIGdpdmUgdG8gdGhlIHByb3BlcnR5LgogKi8KIHR5cGVkZWYgc3RydWN0IHsKLSAg ICBjb25zdCBjaGFyKiBjb25zdCBuYW1lOworICAgIGNvbnN0IGNoYXIqIG5hbWU7CiAgICAgSlNP YmplY3RDYWxsQXNGdW5jdGlvbkNhbGxiYWNrIGNhbGxBc0Z1bmN0aW9uOwogICAgIEpTUHJvcGVy dHlBdHRyaWJ1dGVzIGF0dHJpYnV0ZXM7CiB9IEpTU3RhdGljRnVuY3Rpb247CkluZGV4OiBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvQVBJL3Rlc3RzL3Rlc3RhcGkuYwo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvQVBJL3Rlc3RzL3Rlc3RhcGkuYwkocmV2aXNpb24gODgxOTcpCisr KyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJL3Rlc3RzL3Rlc3RhcGkuYwkod29ya2luZyBjb3B5 KQpAQCAtOTM3LDYgKzkzNywxNCBAQCBzdGF0aWMgYm9vbCBjaGVja0ZvckN5Y2xlSW5Qcm90b3R5 cGVDaGFpCiAgICAgcmV0dXJuIHJlc3VsdDsKIH0KIAorc3RhdGljIHZvaWQgY2hlY2tDb25zdG5l c3NJbkpTT2JqZWN0TmFtZXMoKQoreworICAgIEpTU3RhdGljRnVuY3Rpb24gKmZ1bjsKKyAgICBm dW4tPm5hbWUgPSAic29tZXRoaW5nIjsKKyAgICBKU1N0YXRpY1ZhbHVlICp2YWw7CisgICAgdmFs LT5uYW1lID0gInNvbWV0aGluZyI7Cit9CisKIGludCBtYWluKGludCBhcmdjLCBjaGFyKiBhcmd2 W10pCiB7CiAjaWYgT1MoV0lORE9XUykKQEAgLTEyNTQsNiArMTI2Miw4IEBAIGludCBtYWluKGlu dCBhcmdjLCBjaGFyKiBhcmd2W10pCiAgICAgYXNzZXJ0RXF1YWxzQXNVVEY4U3RyaW5nKGpzQ0ZF bXB0eVN0cmluZywgIiIpOwogICAgIGFzc2VydEVxdWFsc0FzVVRGOFN0cmluZyhqc0NGRW1wdHlT dHJpbmdXaXRoQ2hhcmFjdGVycywgIiIpOwogICAgIAorICAgIGNoZWNrQ29uc3RuZXNzSW5KU09i amVjdE5hbWVzKCk7CisgICAgCiAgICAgQVNTRVJUKEpTVmFsdWVJc1N0cmljdEVxdWFsKGNvbnRl eHQsIGpzVHJ1ZSwganNUcnVlKSk7CiAgICAgQVNTRVJUKCFKU1ZhbHVlSXNTdHJpY3RFcXVhbChj b250ZXh0LCBqc09uZSwganNPbmVTdHJpbmcpKTsK 96309 2011-06-07 15:20:37 -0700 2011-06-07 19:00:44 -0700 proposed patch patch1.txt text/plain 2477 j.mont SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gODgyNTIpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAK KzIwMTEtMDYtMDcgIEp1YW4gQy4gTW9udGVtYXlvciAgPGptb250QGFwcGxlLmNvbT4KKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNYWtlIEpTU3RhdGlj RnVuY3Rpb24gYW5kIEpTU3RhdGljVmFsdWUgbGVzcyAiY29uc3QiCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MjIyMgorCisgICAgICAgICogQVBJL0pT T2JqZWN0UmVmLmg6CisgICAgICAgICogQVBJL3Rlc3RzL3Rlc3RhcGkuYzoKKyAgICAgICAgKGNo ZWNrQ29uc3RuZXNzSW5KU09iamVjdE5hbWVzKToKKyAgICAgICAgKG1haW4pOgorICAgICAgICAq IEphdmFTY3JpcHRDb3JlLnhjb2RlcHJvai9wcm9qZWN0LnBieHByb2o6CisKIDIwMTEtMDYtMDYg IEphbWVzIFNpbW9uc2VuICA8c2ltb25qYW1AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmll d2VkIGJ5IEphbWVzIFJvYmluc29uLgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL0FQSS9K U09iamVjdFJlZi5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9BUEkvSlNP YmplY3RSZWYuaAkocmV2aXNpb24gODgyNTIpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJ L0pTT2JqZWN0UmVmLmgJKHdvcmtpbmcgY29weSkKQEAgLTI4OCw3ICsyODgsNyBAQCB0eXBlZGVm IEpTVmFsdWVSZWYKIEBmaWVsZCBhdHRyaWJ1dGVzIEEgbG9naWNhbGx5IE9SZWQgc2V0IG9mIEpT UHJvcGVydHlBdHRyaWJ1dGVzIHRvIGdpdmUgdG8gdGhlIHByb3BlcnR5LgogKi8KIHR5cGVkZWYg c3RydWN0IHsKLSAgICBjb25zdCBjaGFyKiBjb25zdCBuYW1lOworICAgIGNvbnN0IGNoYXIqIG5h bWU7CiAgICAgSlNPYmplY3RHZXRQcm9wZXJ0eUNhbGxiYWNrIGdldFByb3BlcnR5OwogICAgIEpT T2JqZWN0U2V0UHJvcGVydHlDYWxsYmFjayBzZXRQcm9wZXJ0eTsKICAgICBKU1Byb3BlcnR5QXR0 cmlidXRlcyBhdHRyaWJ1dGVzOwpAQCAtMzAyLDcgKzMwMiw3IEBAIHR5cGVkZWYgc3RydWN0IHsK IEBmaWVsZCBhdHRyaWJ1dGVzIEEgbG9naWNhbGx5IE9SZWQgc2V0IG9mIEpTUHJvcGVydHlBdHRy aWJ1dGVzIHRvIGdpdmUgdG8gdGhlIHByb3BlcnR5LgogKi8KIHR5cGVkZWYgc3RydWN0IHsKLSAg ICBjb25zdCBjaGFyKiBjb25zdCBuYW1lOworICAgIGNvbnN0IGNoYXIqIG5hbWU7CiAgICAgSlNP YmplY3RDYWxsQXNGdW5jdGlvbkNhbGxiYWNrIGNhbGxBc0Z1bmN0aW9uOwogICAgIEpTUHJvcGVy dHlBdHRyaWJ1dGVzIGF0dHJpYnV0ZXM7CiB9IEpTU3RhdGljRnVuY3Rpb247CkluZGV4OiBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvQVBJL3Rlc3RzL3Rlc3RhcGkuYwo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvQVBJL3Rlc3RzL3Rlc3RhcGkuYwkocmV2aXNpb24gODgyNTIpCisr KyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQVBJL3Rlc3RzL3Rlc3RhcGkuYwkod29ya2luZyBjb3B5 KQpAQCAtOTM3LDYgKzkzNywxNCBAQCBzdGF0aWMgYm9vbCBjaGVja0ZvckN5Y2xlSW5Qcm90b3R5 cGVDaGFpCiAgICAgcmV0dXJuIHJlc3VsdDsKIH0KIAorc3RhdGljIHZvaWQgY2hlY2tDb25zdG5l c3NJbkpTT2JqZWN0TmFtZXMoKQoreworICAgIEpTU3RhdGljRnVuY3Rpb24gZnVuOworICAgIGZ1 bi5uYW1lID0gInNvbWV0aGluZyI7CisgICAgSlNTdGF0aWNWYWx1ZSB2YWw7CisgICAgdmFsLm5h bWUgPSAic29tZXRoaW5nIjsKK30KKwogaW50IG1haW4oaW50IGFyZ2MsIGNoYXIqIGFyZ3ZbXSkK IHsKICNpZiBPUyhXSU5ET1dTKQpAQCAtMTI1NCw2ICsxMjYyLDggQEAgaW50IG1haW4oaW50IGFy Z2MsIGNoYXIqIGFyZ3ZbXSkKICAgICBhc3NlcnRFcXVhbHNBc1VURjhTdHJpbmcoanNDRkVtcHR5 U3RyaW5nLCAiIik7CiAgICAgYXNzZXJ0RXF1YWxzQXNVVEY4U3RyaW5nKGpzQ0ZFbXB0eVN0cmlu Z1dpdGhDaGFyYWN0ZXJzLCAiIik7CiAgICAgCisgICAgY2hlY2tDb25zdG5lc3NJbkpTT2JqZWN0 TmFtZXMoKTsKKyAgICAKICAgICBBU1NFUlQoSlNWYWx1ZUlzU3RyaWN0RXF1YWwoY29udGV4dCwg anNUcnVlLCBqc1RydWUpKTsKICAgICBBU1NFUlQoIUpTVmFsdWVJc1N0cmljdEVxdWFsKGNvbnRl eHQsIGpzT25lLCBqc09uZVN0cmluZykpOwo=