396936 - FLAGP_TO_THING not typesafe in C++

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Benjamin Smedberg]

Attachment: Fix FLAGP_TO_THING, rev. 1

Uses of FLAGP_TO_THING macro don't compile in C++ unless you cast the result to JSGCThing*. But since all uses actually want a JSGCThing*, we should just make the macro do the work. This fixes a mozilla-central compile error intoduced by bug 394709

Comment 1

[Brendan Eich [:brendan]]

Comment on attachment 281696 [details] [diff] [review]
Fix FLAGP_TO_THING, rev. 1

>diff -r ec7c5dd6bb66 js/src/jsgc.cpp
>--- a/js/src/jsgc.cpp	Thu Sep 20 14:40:41 2007 -0400
>+++ b/js/src/jsgc.cpp	Thu Sep 20 14:45:26 2007 -0400
>@@ -336,7 +336,7 @@ static JSBool js_gcUseMmap = JS_FALSE;
> #define FLAGP_TO_THING(flagp, thingSize)                                      \
>     (JS_ASSERT(((jsuword) (flagp) & GC_ARENA_MASK) >=                         \
>                (ARENA_INFO_OFFSET - THINGS_PER_ARENA(thingSize))),            \
>-     (void *)(((jsuword) (flagp) & ~GC_ARENA_MASK) +                          \
>+     (JSGCThing *)(((jsuword) (flagp) & ~GC_ARENA_MASK) +                     \
>               (thingSize) * FLAGP_TO_INDEX(flagp)))

Indent the overflow line to underhang the left operand of + (so the (thingSize) * ... starts in the same column as the second left parenthesis after the cast on the first line).

Igor is really the guy to r+ this but I think he won't mind if I do. Leaving addl. review set but this can go in with the style fix ASAP.

/be

Comment 2

[Brendan Eich [:brendan]]

Type safety is not the issue, since the FLAGP_TO_THING macro hides a C-style cast based on address arithmetic magic. But it's good to consolidate the (JSGCThing *) cast, and avoid expanding it in 3 of 4 call sites.

/be

Comment 3

[Benjamin Smedberg]

Fixed on CVS trunk.