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

--HG--
extra : rebase_source : 15060321f567816ca434cdf1ef816d8322ceefff

js/src/builtin/TestingFunctions.cpp

@@ -2841,14 +2841,13 @@ GetBacktrace(JSContext* cx, unsigned argc, Value* vp)
         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)))
+    if (!(str = JS_NewStringCopyZ(cx, buf.get())))
         return false;
-    JS_smprintf_free(buf);
 
     args.rval().setString(str);
     return true;

js/src/jsfriendapi.cpp

@@ -784,14 +784,14 @@ FormatValue(JSContext* cx, const Value& vArg, JSAutoByteString& bytes)
 
 // 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) {
@@ -802,8 +802,8 @@ sprintf_append(JSContext* cx, char* buf, const char* fmt, ...)
     return result;
 }
 
-static char*
+static JS::UniqueChars
-FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
+FormatFrame(JSContext* cx, const FrameIter& iter, JS::UniqueChars&& inBuf, int num,
             bool showArgs, bool showLocals, bool showThisProps)
 {
     MOZ_ASSERT(!cx->isExceptionPending());
@@ -831,16 +831,17 @@ FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
     }
 
     // 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;
@@ -889,7 +890,7 @@ FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
             }
 
             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 ? " = " : "",
@@ -901,7 +902,7 @@ FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
 
                 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;
@@ -911,7 +912,7 @@ FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
     }
 
     // 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);
@@ -936,9 +937,9 @@ FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
                 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;
@@ -965,7 +966,7 @@ FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
                 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;
@@ -989,13 +990,13 @@ FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
             }
 
             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)
@@ -1007,22 +1008,23 @@ FormatFrame(JSContext* cx, const FrameIter& iter, char* buf, int num,
     return buf;
 }
 
-static char*
+static JS::UniqueChars
-FormatWasmFrame(JSContext* cx, const FrameIter& iter, char* buf, int num, bool showArgs)
+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()",
+    JS::UniqueChars buf = sprintf_append(cx, Move(inBuf), "%d %s()",
-                         num,
+                                         num,
-                         nameStr ? nameStr.get() : "<wasm-function>");
+                                         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);
 
@@ -1030,23 +1032,25 @@ FormatWasmFrame(JSContext* cx, const FrameIter& iter, char* buf, int num, bool s
     return buf;
 }
 
-JS_FRIEND_API(char*)
+JS_FRIEND_API(JS::UniqueChars)
-JS::FormatStackDump(JSContext* cx, char* buf, bool showArgs, bool showLocals, bool showThisProps)
+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;
 }

js/src/jsfriendapi.h

@@ -267,8 +267,9 @@ DumpBacktrace(JSContext* cx);
 namespace JS {
 
 /** Exposed for DumpJSStack */
-extern JS_FRIEND_API(char*)
+extern JS_FRIEND_API(JS::UniqueChars)
-FormatStackDump(JSContext* cx, char* buf, bool showArgs, bool showLocals, bool showThisProps);
+FormatStackDump(JSContext* cx, JS::UniqueChars&& buf, bool showArgs, bool showLocals,
+                bool showThisProps);
 
 /**
  * Set all of the uninitialized lexicals on an object to undefined. Return

js/src/shell/js.cpp

@@ -4205,13 +4205,12 @@ StackDump(JSContext* cx, unsigned argc, Value* vp)
     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);
+        fputs(buf.get(), gOutFile->fp);
-        JS_smprintf_free(buf);
     }
 
     args.rval().setUndefined();

js/xpconnect/src/XPCDebug.cpp

@@ -41,20 +41,19 @@ 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)) {
+    } else if (JS::UniqueChars buf = xpc_PrintJSStack(cx, showArgs, showLocals, showThisProps)) {
-        DebugDump("%s\n", buf);
+        DebugDump("%s\n", buf.get());
-        JS_smprintf_free(buf);
     }
     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");
 

js/xpconnect/src/nsXPConnect.cpp

@@ -1009,7 +1009,7 @@ nsXPConnect::DebugPrintJSStack(bool showArgs,
     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;
 }

js/xpconnect/src/xpcprivate.h

@@ -2442,9 +2442,8 @@ 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
+// current JS stack.
-// string with JS_smprintf_free().
+extern JS::UniqueChars
-extern char*
 xpc_PrintJSStack(JSContext* cx, bool showArgs, bool showLocals,
                  bool showThisProps);