1381727 - Remove JS_smprintf_free

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

Can just use js_free directly, I'd think.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove JS_smprintf_free()

Because it's just a wrapper for js_free().

Furthermore, JS_smprintf() and friends return a JS::UniqueChars, which is also
wired up to free with js_free(). So having the indirection via
JS_smprintf_free() obfuscates things.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Remove SmprintfFree()

It's just a complex wrapper for free(), or equivalent function. (In practice,
all the uses end up in free().)

Comment 3

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8889284 [details] [diff] [review]
(part 1) - Remove JS_smprintf_free()

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

::: dom/power/PowerManagerService.cpp
@@ +28,4 @@
>  #include <android/log.h>
>  extern "C" char* PrintJSStack();
>  static void LogFunctionAndJSStack(const char* funcname) {
>    char *jsstack = PrintJSStack();

This makes me wonder whether PrintJSStack could return a UniqueChars. But maybe that would make it more annoying to use from a debugger or something; I wasn't aware of PrintJSStack.

::: js/xpconnect/src/xpcprivate.h
@@ +1729,5 @@
>      }
>  
>      inline void SweepTearOffs();
>  
> +    // Returns a string that shuld be free'd using js_free (or null).

While you're here, s/shuld/should/. And while I'm being pedantic, maybe reword this to something like

  // Returns a string that should be freed using js_free, or nullptr on failure.

(Reading the comment, it sounds like I can either do js_free(s), or null(s).

Comment 4

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/edded26e7141609581dfabd39298af699ab3abd6
Bug 1381727 (part 1) - Remove JS_smprintf_free(). r=sfink.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b441867efe7020c53e344e53200bc2915e854487
Bug 1381727 (part 2) - Remove SmprintfFree(). r=glandium.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/edded26e7141
https://hg.mozilla.org/mozilla-central/rev/b441867efe70