Bug 1334278 - have FormatStackDump return UniqueChars; r?froydnj draft
authorTom Tromey <tom@tromey.com>
Fri, 21 Apr 2017 12:47:06 -0600
changeset 568735 d8c53d11dd277f4f5ddd3e74a971957c332021be
parent 568734 6bc390539e800a48281e77a04cfb727696b9cbf1
child 626010 1cdceda8f298912b5284de174499dce31f2c4394
push id55961
push userbmo:ttromey@mozilla.com
push dateWed, 26 Apr 2017 14:11:48 +0000
reviewersfroydnj
bugs1334278
milestone55.0a1
Bug 1334278 - have FormatStackDump return UniqueChars; r?froydnj Change FormatStackDump to return UniqueChars and fix up the users. This removes a bit more manual memory management. MozReview-Commit-ID: 60GBgeS4rzg
js/src/builtin/TestingFunctions.cpp
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/src/shell/js.cpp
js/xpconnect/src/XPCDebug.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -2836,24 +2836,23 @@ GetBacktrace(JSContext* cx, unsigned arg
             return false;
         showLocals = ToBoolean(v);
 
         if (!JS_GetProperty(cx, cfg, "thisprops", &v))
             return false;
         showThisProps = ToBoolean(v);
     }
 
-    char* buf = JS::FormatStackDump(cx, nullptr, showArgs, showLocals, showThisProps);
+    JS::UniqueChars buf = JS::FormatStackDump(cx, nullptr, showArgs, showLocals, showThisProps);
     if (!buf)
         return false;
 
     RootedString str(cx);
-    if (!(str = JS_NewStringCopyZ(cx, buf)))
-        return false;
-    JS_smprintf_free(buf);
+    if (!(str = JS_NewStringCopyZ(cx, buf.get())))
+        return false;
 
     args.rval().setString(str);
     return true;
 }
 
 static bool
 ReportOutOfMemory(JSContext* cx, unsigned argc, Value* vp)
 {
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -779,36 +779,36 @@ FormatValue(JSContext* cx, const Value& 
     const char* found = strstr(buf, "function ");
     if (found && (found - buf <= 2))
         return "[function]";
     return buf;
 }
 
 // Wrapper for JS_sprintf_append() that reports allocation failure to the
 // context.
-static char*
+static JS::UniqueChars
 MOZ_FORMAT_PRINTF(3, 4)
-sprintf_append(JSContext* cx, char* buf, const char* fmt, ...)
+sprintf_append(JSContext* cx, JS::UniqueChars&& buf, const char* fmt, ...)
 {
     va_list ap;
 
     va_start(ap, fmt);
-    char* result = JS_vsprintf_append(UniqueChars(buf), fmt, ap).release();
+    JS::UniqueChars result = JS_vsprintf_append(Move(buf), fmt, ap);
     va_end(ap);
 
     if (!result) {
         ReportOutOfMemory(cx);
         return nullptr;
     }
 
     return result;
 }
 
-static char*
-FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
+static JS::UniqueChars
+FormatFrame(JSContext* cx, const FrameIter& iter, JS::UniqueChars&& inBuf, int num,
             bool showArgs, bool showLocals, bool showThisProps)
 {
     MOZ_ASSERT(!cx->isExceptionPending());
     RootedScript script(cx, iter.script());
     jsbytecode* pc = iter.pc();
 
     RootedObject envChain(cx, iter.environmentChain(cx));
     JSAutoCompartment ac(cx, envChain);
@@ -826,26 +826,27 @@ FormatFrame(JSContext* cx, const FrameIt
         fun && !fun->isArrow() && !fun->isDerivedClassConstructor() &&
         !(fun->isBoundFunction() && iter.isConstructing()))
     {
         if (!GetFunctionThis(cx, iter.abstractFramePtr(), &thisVal))
             return nullptr;
     }
 
     // print the frame number and function name
+    JS::UniqueChars buf(Move(inBuf));
     if (funname) {
         JSAutoByteString funbytes;
         char* str = funbytes.encodeLatin1(cx, funname);
         if (!str)
             return nullptr;
-        buf = sprintf_append(cx, buf, "%d %s(", num, str);
+        buf = sprintf_append(cx, Move(buf), "%d %s(", num, str);
     } else if (fun) {
-        buf = sprintf_append(cx, buf, "%d anonymous(", num);
+        buf = sprintf_append(cx, Move(buf), "%d anonymous(", num);
     } else {
-        buf = sprintf_append(cx, buf, "%d <TOP LEVEL>", num);
+        buf = sprintf_append(cx, Move(buf), "%d <TOP LEVEL>", num);
     }
     if (!buf)
         return nullptr;
 
     if (showArgs && iter.hasArgs()) {
         PositionalFormalParameterIter fi(script);
         bool first = true;
         for (unsigned i = 0; i < iter.numActualArgs(); i++) {
@@ -884,39 +885,39 @@ FormatFrame(JSContext* cx, const FrameIt
                         return nullptr;
                 } else {
                     name = "(destructured parameter)";
                 }
                 fi++;
             }
 
             if (value) {
-                buf = sprintf_append(cx, buf, "%s%s%s%s%s%s",
+                buf = sprintf_append(cx, Move(buf), "%s%s%s%s%s%s",
                                      !first ? ", " : "",
                                      name ? name :"",
                                      name ? " = " : "",
                                      arg.isString() ? "\"" : "",
                                      value,
                                      arg.isString() ? "\"" : "");
                 if (!buf)
                     return nullptr;
 
                 first = false;
             } else {
-                buf = sprintf_append(cx, buf,
+                buf = sprintf_append(cx, Move(buf),
                                      "    <Failed to get argument while inspecting stack frame>\n");
                 if (!buf)
                     return nullptr;
 
             }
         }
     }
 
     // print filename and line number
-    buf = sprintf_append(cx, buf, "%s [\"%s\":%d]\n",
+    buf = sprintf_append(cx, Move(buf), "%s [\"%s\":%d]\n",
                          fun ? ")" : "",
                          filename ? filename : "<unknown>",
                          lineno);
     if (!buf)
         return nullptr;
 
 
     // Note: Right now we don't dump the local variables anymore, because
@@ -931,19 +932,19 @@ FormatFrame(JSContext* cx, const FrameIt
                 if (cx->isThrowingOutOfMemory())
                     return nullptr;
                 cx->clearPendingException();
             }
             if (thisValStr) {
                 const char* str = thisValBytes.encodeLatin1(cx, thisValStr);
                 if (!str)
                     return nullptr;
-                buf = sprintf_append(cx, buf, "    this = %s\n", str);
+                buf = sprintf_append(cx, Move(buf), "    this = %s\n", str);
             } else {
-                buf = sprintf_append(cx, buf, "    <failed to get 'this' value>\n");
+                buf = sprintf_append(cx, Move(buf), "    <failed to get 'this' value>\n");
             }
             if (!buf)
                 return nullptr;
         }
     }
 
     if (showThisProps && thisVal.isObject()) {
         RootedObject obj(cx, &thisVal.toObject());
@@ -960,17 +961,17 @@ FormatFrame(JSContext* cx, const FrameIt
             RootedId id(cx, keys[i]);
             RootedValue key(cx, IdToValue(id));
             RootedValue v(cx);
 
             if (!GetProperty(cx, obj, obj, id, &v)) {
                 if (cx->isThrowingOutOfMemory())
                     return nullptr;
                 cx->clearPendingException();
-                buf = sprintf_append(cx, buf,
+                buf = sprintf_append(cx, Move(buf),
                                      "    <Failed to fetch property while inspecting stack frame>\n");
                 if (!buf)
                     return nullptr;
                 continue;
             }
 
             JSAutoByteString nameBytes;
             const char* name = FormatValue(cx, key, nameBytes);
@@ -984,74 +985,77 @@ FormatFrame(JSContext* cx, const FrameIt
             const char* value = FormatValue(cx, v, valueBytes);
             if (!value) {
                 if (cx->isThrowingOutOfMemory())
                     return nullptr;
                 cx->clearPendingException();
             }
 
             if (name && value) {
-                buf = sprintf_append(cx, buf, "    this.%s = %s%s%s\n",
+                buf = sprintf_append(cx, Move(buf), "    this.%s = %s%s%s\n",
                                      name,
                                      v.isString() ? "\"" : "",
                                      value,
                                      v.isString() ? "\"" : "");
             } else {
-                buf = sprintf_append(cx, buf,
+                buf = sprintf_append(cx, Move(buf),
                                      "    <Failed to format values while inspecting stack frame>\n");
             }
             if (!buf)
                 return nullptr;
         }
     }
 
     MOZ_ASSERT(!cx->isExceptionPending());
     return buf;
 }
 
-static char*
-FormatWasmFrame(JSContext* cx, const FrameIter& iter, char* buf, int num, bool showArgs)
+static JS::UniqueChars
+FormatWasmFrame(JSContext* cx, const FrameIter& iter, JS::UniqueChars&& inBuf, int num,
+                bool showArgs)
 {
     JSAtom* functionDisplayAtom = iter.functionDisplayAtom();
     UniqueChars nameStr;
     if (functionDisplayAtom)
         nameStr = StringToNewUTF8CharsZ(cx, *functionDisplayAtom);
 
-    buf = sprintf_append(cx, buf, "%d %s()",
-                         num,
-                         nameStr ? nameStr.get() : "<wasm-function>");
+    JS::UniqueChars buf = sprintf_append(cx, Move(inBuf), "%d %s()",
+                                         num,
+                                         nameStr ? nameStr.get() : "<wasm-function>");
     if (!buf)
         return nullptr;
     const char* filename = iter.filename();
     uint32_t lineno = iter.computeLine();
-    buf = sprintf_append(cx, buf, " [\"%s\":%d]\n",
+    buf = sprintf_append(cx, Move(buf), " [\"%s\":%d]\n",
                          filename ? filename : "<unknown>",
                          lineno);
 
     MOZ_ASSERT(!cx->isExceptionPending());
     return buf;
 }
 
-JS_FRIEND_API(char*)
-JS::FormatStackDump(JSContext* cx, char* buf, bool showArgs, bool showLocals, bool showThisProps)
+JS_FRIEND_API(JS::UniqueChars)
+JS::FormatStackDump(JSContext* cx, JS::UniqueChars&& inBuf, bool showArgs, bool showLocals,
+                    bool showThisProps)
 {
     int num = 0;
 
+    JS::UniqueChars buf(Move(inBuf));
     for (AllFramesIter i(cx); !i.done(); ++i) {
         if (i.hasScript())
-            buf = FormatFrame(cx, i, buf, num, showArgs, showLocals, showThisProps);
+            buf = FormatFrame(cx, i, Move(buf), num, showArgs, showLocals, showThisProps);
         else
-            buf = FormatWasmFrame(cx, i, buf, num, showArgs);
+            buf = FormatWasmFrame(cx, i, Move(buf), num, showArgs);
         if (!buf)
             return nullptr;
         num++;
     }
 
     if (!num)
-        buf = JS_sprintf_append(UniqueChars(buf), "JavaScript stack is empty\n").release();
+        buf = JS_sprintf_append(Move(buf), "JavaScript stack is empty\n");
 
     return buf;
 }
 
 extern JS_FRIEND_API(bool)
 JS::ForceLexicalInitialization(JSContext *cx, HandleObject obj)
 {
     AssertHeapIsIdle();
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -262,18 +262,19 @@ DumpBacktrace(JSContext* cx, FILE* fp);
 extern JS_FRIEND_API(void)
 DumpBacktrace(JSContext* cx);
 
 } // namespace js
 
 namespace JS {
 
 /** Exposed for DumpJSStack */
-extern JS_FRIEND_API(char*)
-FormatStackDump(JSContext* cx, char* buf, bool showArgs, bool showLocals, bool showThisProps);
+extern JS_FRIEND_API(JS::UniqueChars)
+FormatStackDump(JSContext* cx, JS::UniqueChars&& buf, bool showArgs, bool showLocals,
+                bool showThisProps);
 
 /**
  * Set all of the uninitialized lexicals on an object to undefined. Return
  * true if any lexicals were initialized and false otherwise.
  * */
 extern JS_FRIEND_API(bool)
 ForceLexicalInitialization(JSContext *cx, HandleObject obj);
 
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -4200,23 +4200,22 @@ StackDump(JSContext* cx, unsigned argc, 
         JS_ReportErrorASCII(cx, "output file is closed");
         return false;
     }
 
     bool showArgs = ToBoolean(args.get(0));
     bool showLocals = ToBoolean(args.get(1));
     bool showThisProps = ToBoolean(args.get(2));
 
-    char* buf = JS::FormatStackDump(cx, nullptr, showArgs, showLocals, showThisProps);
+    JS::UniqueChars buf = JS::FormatStackDump(cx, nullptr, showArgs, showLocals, showThisProps);
     if (!buf) {
         fputs("Failed to format JavaScript stack for dump\n", gOutFile->fp);
         JS_ClearPendingException(cx);
     } else {
-        fputs(buf, gOutFile->fp);
-        JS_smprintf_free(buf);
+        fputs(buf.get(), gOutFile->fp);
     }
 
     args.rval().setUndefined();
     return true;
 }
 #endif
 
 static bool
--- a/js/xpconnect/src/XPCDebug.cpp
+++ b/js/xpconnect/src/XPCDebug.cpp
@@ -36,28 +36,27 @@ static void DebugDump(const char* fmt, .
 }
 
 bool
 xpc_DumpJSStack(bool showArgs, bool showLocals, bool showThisProps)
 {
     JSContext* cx = nsContentUtils::GetCurrentJSContextForThread();
     if (!cx) {
         printf("there is no JSContext on the stack!\n");
-    } else if (char* buf = xpc_PrintJSStack(cx, showArgs, showLocals, showThisProps)) {
-        DebugDump("%s\n", buf);
-        JS_smprintf_free(buf);
+    } else if (JS::UniqueChars buf = xpc_PrintJSStack(cx, showArgs, showLocals, showThisProps)) {
+        DebugDump("%s\n", buf.get());
     }
     return true;
 }
 
-char*
+JS::UniqueChars
 xpc_PrintJSStack(JSContext* cx, bool showArgs, bool showLocals,
                  bool showThisProps)
 {
     JS::AutoSaveExceptionState state(cx);
 
-    char* buf = JS::FormatStackDump(cx, nullptr, showArgs, showLocals, showThisProps);
+    JS::UniqueChars buf = JS::FormatStackDump(cx, nullptr, showArgs, showLocals, showThisProps);
     if (!buf)
         DebugDump("%s", "Failed to format JavaScript stack for dump\n");
 
     state.restore();
     return buf;
 }
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -1004,17 +1004,17 @@ char*
 nsXPConnect::DebugPrintJSStack(bool showArgs,
                                bool showLocals,
                                bool showThisProps)
 {
     JSContext* cx = nsContentUtils::GetCurrentJSContext();
     if (!cx)
         printf("there is no JSContext on the nsIThreadJSContextStack!\n");
     else
-        return xpc_PrintJSStack(cx, showArgs, showLocals, showThisProps);
+        return xpc_PrintJSStack(cx, showArgs, showLocals, showThisProps).release();
 
     return nullptr;
 }
 
 NS_IMETHODIMP
 nsXPConnect::VariantToJS(JSContext* ctx, JSObject* scopeArg, nsIVariant* value,
                          MutableHandleValue _retval)
 {
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -2437,19 +2437,18 @@ xpc_JSObjectIsID(JSContext* cx, JSObject
 
 /***************************************************************************/
 // in XPCDebug.cpp
 
 extern bool
 xpc_DumpJSStack(bool showArgs, bool showLocals, bool showThisProps);
 
 // Return a newly-allocated string containing a representation of the
-// current JS stack.  It is the *caller's* responsibility to free this
-// string with JS_smprintf_free().
-extern char*
+// current JS stack.
+extern JS::UniqueChars
 xpc_PrintJSStack(JSContext* cx, bool showArgs, bool showLocals,
                  bool showThisProps);
 
 /******************************************************************************
  * Handles pre/post script processing.
  */
 class MOZ_RAII AutoScriptEvaluate
 {