Closed Bug 969798 Opened 6 years ago Closed 6 years ago

Convert JS_ConvertArguments APIs to take CallArgs rather than raw Value pointer

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(2 files)

At the moment these functions take argc and argv arguments and assume the Values pointed to are rooted.  Let's make this more explicit and make these take a CallArgs& instead.
Attachment #8372796 - Flags: review?(terrence)
I think we should just remove that function and replace it with explicit JS::ToXXX functions.
Attached patch remove-converSplinter Review
Compiles, but untested.
Comment on attachment 8372796 [details] [diff] [review]
convert-convert-args

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

Looks good, but we should try to at least lift the CallArgs decls up to make future CallArgsification easier. r=me

::: js/src/jsapi.cpp
@@ +244,5 @@
>              }
>              break;
>          }
> +        MutableHandleValue arg = args[index++];
> +        //RootedValue arg(cx, *sp);

Guess you forgot to delete this line.

::: js/src/perf/jsperf.cpp
@@ +171,5 @@
>  static bool
>  pm_construct(JSContext* cx, unsigned argc, jsval* vp)
>  {
>      uint32_t mask;
> +    if (!JS_ConvertArguments(cx, CallArgsFromVp(argc, vp), "u", &mask))

(1) Please move the CallArgs to the top and fix the two other uses of vp while you're here. (2) This is a totally absurd way to call ToUInt32, so we should just call ToUInt32 here.

::: js/src/shell/js.cpp
@@ +2481,5 @@
>  EvalInContext(JSContext *cx, unsigned argc, jsval *vp)
>  {
>      RootedString str(cx);
>      RootedObject sobj(cx);
> +    if (!JS_ConvertArguments(cx, CallArgsFromVp(argc, vp), "S / o", str.address(), sobj.address()))

Please move the CallArgs to the top and fix the two other uses of vp while you're here.

::: netwerk/base/src/ProxyAutoConfig.cpp
@@ +400,5 @@
>      return false;
>    }
>  
>    JS::Rooted<JSString*> arg1(cx);
> +  if (!JS_ConvertArguments(cx, JS::CallArgsFromVp(argc, vp), "S", arg1.address()))

Move CallArgs to the top and fix the other uses here. In particular, you should be able to replace arg1 and save a root.

@@ +441,5 @@
>  static
>  bool PACProxyAlert(JSContext *cx, unsigned int argc, JS::Value *vp)
>  {
>    JS::Rooted<JSString*> arg1(cx);
> +  if (!JS_ConvertArguments(cx, JS::CallArgsFromVp(argc, vp), "S", arg1.address()))

Ditto.
Attachment #8372796 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/cae585897f68
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.