Closed Bug 871192 Opened 11 years ago Closed 11 years ago

Fix more rooting hazards in the JS debugger

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

Details

Attachments

(1 file)

Attached patch Patch (v1)Splinter Review
      No description provided.
Attachment #748483 - Flags: review?(tschneidereit)
Comment on attachment 748483 [details] [diff] [review]
Patch (v1)

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

Nice, thanks!

::: js/jsd/jsd_scpt.cpp
@@ +59,5 @@
>  
>  static JSDScript*
>  _newJSDScript(JSDContext*  jsdc,
>                JSContext    *cx,
> +              JSScript     *scrpt)

At least inside of js/src, we append a "_" to the name instead of mangling it like this. I think I'd prefer that in jsd, too.

@@ +281,5 @@
>  
>  JSDScript *
>  jsd_FindOrCreateJSDScript(JSDContext    *jsdc,
>                            JSContext     *cx,
> +                          JSScript      *scrpt,

See above

@@ +673,5 @@
>  
>  void
>  jsd_DestroyScriptHookProc( 
>                  JSFreeOp    *fop,
> +                JSScript    *scrpt,

same

@@ +777,3 @@
>                  jsval closure)
>  {
> +    JS::RootedScript script(cx, scrpt);

same

::: js/jsd/jsd_xpc.cpp
@@ +994,5 @@
>  jsdScript::CreatePPLineMap()
>  {    
>      JSContext  *cx  = JSD_GetDefaultJSContext (mCx);
>      JSAutoRequest ar(cx);
> +    JS::RootedObject obj(cx, JS_NewObject(cx, NULL, NULL, NULL));

Please add a NULL check for the newly created object, while you're already in here.
Attachment #748483 - Flags: review?(tschneidereit) → review+
https://hg.mozilla.org/mozilla-central/rev/87fc70f25ef4
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: