817452 - Remove AutoRelease[Nullable]Ptr

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[:Benjamin Peterson]

We have way too many of these scoped release/delete thingies.

Comment 1

[:Benjamin Peterson]

Attachment: rm

Comment 2

[Nicholas Nethercote [inactive]]

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.

Comment 3

[:Benjamin Peterson]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f5477b99877f

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

Comment 4

[Nicholas Nethercote [inactive]]

> 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

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f5477b99877f