946947 - getLineNumberOffsets requires a numeric number

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

I was passing it the value that the user typed in, which was of course a string.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Convert getLineNumberOffsets param to a number instead of insisting on a number

I don't know if you want this, and I didn't bother to test it at all, and really I'm just a jerk. But I felt bad about just complaining about it on IRC and not doing anything.

Comment 2

[Jim Blandy :jimb]

Comment on attachment 8343321 [details] [diff] [review]
Convert getLineNumberOffsets param to a number instead of insisting on a number

Review of attachment 8343321 [details] [diff] [review]:
-----------------------------------------------------------------

I guess I'm surprised there isn't a function that captures all that Value-to-double-to-size_t-check-for-lost-bits dance in a nice package, that we could just call here...

::: js/src/vm/Debugger.cpp
@@ +3384,5 @@
>      THIS_DEBUGSCRIPT_SCRIPT(cx, argc, vp, "getLineOffsets", args, obj, script);
>      REQUIRE_ARGC("Debugger.Script.getLineOffsets", 1);
>  
>      /* Parse lineno argument. */
> +    RootedValue linenoValue(cx, args[0]);

Nit: could we make this local to the block, and put the ToNumber call inside the block, too? Then linenoValue's scope would be clearer, and the block would look less like the if's body.

Comment 3

[Steve Fink [:sfink] [:s:]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/375387aa7835

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/375387aa7835