Closed
Bug 969798
Opened 11 years ago
Closed 11 years ago
Convert JS_ConvertArguments APIs to take CallArgs rather than raw Value pointer
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: jonco, Assigned: jonco)
References
Details
Attachments
(2 files)
11.78 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
16.53 KB,
patch
|
Details | Diff | Splinter Review |
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)
Comment 1•11 years ago
|
||
I think we should just remove that function and replace it with explicit JS::ToXXX functions.
Comment 2•11 years ago
|
||
Compiles, but untested.
Comment 3•11 years ago
|
||
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+
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cae585897f68
Comment 5•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cae585897f68
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•