FLAGP_TO_THING not typesafe in C++

RESOLVED FIXED in mozilla1.9beta1

Status

()

Core
JavaScript Engine
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Benjamin Smedberg, Assigned: Benjamin Smedberg)

Tracking

unspecified
mozilla1.9beta1
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
Created attachment 281696 [details] [diff] [review]
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
Attachment #281696 - Flags: review?(brendan)
Attachment #281696 - Flags: approval1.9?
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
Attachment #281696 - Flags: review?(igor)
Attachment #281696 - Flags: review?(brendan)
Attachment #281696 - Flags: review+
Attachment #281696 - Flags: approval1.9?
Attachment #281696 - Flags: approval1.9+
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
(Assignee)

Updated

11 years ago
Blocks: 392263
No longer blocks: 394709

Updated

11 years ago
Attachment #281696 - Flags: review?(igor) → review+
(Assignee)

Comment 3

11 years ago
Fixed on CVS trunk.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.