Bug 62222 – JSStaticFunction and JSStaticValue are overly "const"

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Juan C. Montemayor]

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;

[Juan C. Montemayor]

Created attachment 96270 [details]
proposed patch

[Darin Adler]

Comment on attachment 96270 [details]
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?

[Darin Adler]

Comment on attachment 96270 [details]
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.

[Geoffrey Garen]

Comment on attachment 96270 [details]
proposed patch

Marking cq+ to get this landed by the commit queue.

[Geoffrey Garen]

Comment on attachment 96270 [details]
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.

[Juan C. Montemayor]

Created attachment 96309 [details]
proposed patch

sorry, didn't catch that in the original report. all fixed now :)

[WebKit Review Bot]

Comment on attachment 96309 [details]
proposed patch

Clearing flags on attachment: 96309

Committed r88313: <http://trac.webkit.org/changeset/88313>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.