Closed
Bug 817452
Opened 13 years ago
Closed 13 years ago
Remove AutoRelease[Nullable]Ptr
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: Benjamin, Assigned: Benjamin)
Details
(Whiteboard: [js:t])
Attachments
(1 file)
5.83 KB,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
We have way too many of these scoped release/delete thingies.
Assignee | ||
Comment 1•13 years ago
|
||
Assignee: general → benjamin
Attachment #687585 -
Flags: review?(jwalden+bmo)
Updated•13 years ago
|
Whiteboard: [js:t]
Assignee | ||
Updated•13 years ago
|
Attachment #687585 -
Flags: review?(jwalden+bmo) → review?(n.nethercote)
![]() |
||
Comment 2•13 years ago
|
||
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+
Assignee | ||
Comment 3•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5477b99877f
I wonder if js::ScopedFreePtr should be renamed js::ScopedFreeJSPtr for clarity.
![]() |
||
Comment 4•13 years ago
|
||
> 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.
Comment 5•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in
before you can comment on or make changes to this bug.
Description
•