579158 - js_XDRAtom creates unnecessary JSString during decoding

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Attachment: fix

bug 579140 comment 8 points out that js_XDRAtom creates an unnecessary JSString for already-atomized strings.  Since atoms may only be strings, it seems the fix is just to fold js_XDRStringAtom into js_XDRAtom.

Comment 1

[Luke Wagner [:luke]]

Oops, wrong dependency.

Comment 2

[Igor Bukanov]

Comment on attachment 457663 [details] [diff] [review]
fix

When you are fixing this it would be nice to rename those jsval_ tags into xdr_ ones to avoid any confusion with jsval internals.

Comment 3

[Luke Wagner [:luke]]

Attachment: fix with nits

Oops, I forgot to address your other requests.

Comment 4

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/caaf823602d6

Comment 5

[Igor Bukanov]

Comment on attachment 457726 [details] [diff] [review]
fix with nits

>-enum jsvaltag {
>+enum XDRValueTag {
>     JSVAL_OBJECT  =             0x0,
>     JSVAL_INT     =             0x1,
>     JSVAL_DOUBLE  =             0x2,
>     JSVAL_STRING  =             0x4,
>     JSVAL_SPECIAL =             0x6,
>     JSVAL_XDRNULL =             0x8,
>     JSVAL_XDRVOID =             0xA
> };

I should have been more clear in comments and stated that the nit was not only about the enum type name but also about constants names. They should be renamed to XDRTAG_OBJECT etc to avoid any resemblance to JSVAL internals and their indexes can be simply 0,1,2 etc. Note that the latter change at this point would need a bumping ox xdr version.

Comment 6

[Luke Wagner [:luke]]

Of course, d'oh!  Will fix.