Closed Bug 817452 Opened 10 years ago Closed 10 years ago

Remove AutoRelease[Nullable]Ptr

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: Benjamin, Assigned: Benjamin)

Details

(Whiteboard: [js:t])

Attachments

(1 file)

We have way too many of these scoped release/delete thingies.
Attached patch rmSplinter Review
Assignee: general → benjamin
Attachment #687585 - Flags: review?(jwalden+bmo)
Whiteboard: [js:t]
Attachment #687585 - Flags: review?(jwalden+bmo) → review?(n.nethercote)
Comment on attachment 687585 [details] [diff] [review]
rm

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

::: js/src/jsexn.cpp
@@ -1149,5 @@
>  
> -    JSExnPrivate *copy = (JSExnPrivate *)cx->malloc_(size);
> -    if (!copy)
> -        return NULL;
> -    AutoReleasePtr autoFreePrivate(copy);

Why did you remove the NULL test?  r=me if you add it back in.

@@ +1155,5 @@
>              return NULL;
>      } else {
>          copy->errorReport = NULL;
>      }
> +    ScopedFreePtr<JSErrorReport> autoFreeErrorReport(copy->errorReport);

I find it really disconcerting that we have mozilla::ScopedFreePtr and js::ScopedFreePtr, which have subtly different behaviour (i.e. one calls free() and the other calls js_free()).

Can you add "js::" to make it clearer, here and everywhere else?  Thanks.
Attachment #687585 - Flags: review?(n.nethercote) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5477b99877f

I wonder if js::ScopedFreePtr should be renamed js::ScopedFreeJSPtr for clarity.
> I wonder if js::ScopedFreePtr should be renamed js::ScopedFreeJSPtr for
> clarity.

The compiler will barf if both versions are in scope... but still, it seems like a recipe for confusion.
https://hg.mozilla.org/mozilla-central/rev/f5477b99877f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.