642449 - GCRoot::GCMember must use the valid() mechanism of RCPtr

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Lars T Hansen]

It's not enough to test for not NULL, we have to test if the pointer value is greater than 1 because of the AVMPLUS_STRING_DELETED trick.  (Or we could change that trick.)

Comment 1

[Lars T Hansen]

Attachment: Patch

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

The tamarin pretxncommit.commit hook (see utils/hooks/tamarin-commit-hook.py) complains about 
trailing whitespace after the period in "whose value is '1'. "

Too bad we cannot refer to AVMPLUS_STRING_DELETED rather than hard-coding the constant 1 and explaining it in a comment.

Aside: Why is AVMPLUS_STRING_DELETED #define'd in AvmCore.h?  AFAICT it is only referenced in AvmCore.cpp.  I will assume its used in the player code.

Comment 3

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 519913 [details] [diff] [review]
Patch

(whoops forgot the R+ with comment 2)

Comment 4

[Lars T Hansen]

(In reply to comment #2)
> The tamarin pretxncommit.commit hook (see utils/hooks/tamarin-commit-hook.py)
> complains about 
> trailing whitespace after the period in "whose value is '1'. "

Noted.  (In general I'm not playing along with that rule...)

> Too bad we cannot refer to AVMPLUS_STRING_DELETED rather than hard-coding the
> constant 1 and explaining it in a comment.

Yes.  Too bad we can't just use a NULL test IMO, but I was just looking for a quick fix here.

> Aside: Why is AVMPLUS_STRING_DELETED #define'd in AvmCore.h?  AFAICT it is
> only referenced in AvmCore.cpp.  I will assume its used in the player code.

Good eye.  It is not used in the player, nor should it be.  I will move the definition into the cpp file.

Comment 5

[Tamarin Bot]

changeset: 6107:c9e697a395c7
user:      Lars T Hansen <[e-mail]>
summary:   Fix 642449 - GCRoot::GCMember must use the valid() mechanism of RCPtr (r=fklockii)

http://hg.mozilla.org/tamarin-redux/rev/c9e697a395c7