bug 617215 - eliminating JS_NewString usage in FF while fixing a leak there. r=bz
authorIgor Bukanov <igor@mir2.org>
Thu, 09 Dec 2010 11:22:15 +0100
changeset 59008 010bd7365328ec688cc934f57cf5d6b360d7756c
parent 59007 4d97e9955bfbfff03d1548db0e1ef7c5f55ad4fe
child 59009 52d20032116aa1ecd79113d0413d2f83ae9400d5
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersbz
bugs617215
milestone2.0b8pre
bug 617215 - eliminating JS_NewString usage in FF while fixing a leak there. r=bz
js/src/jsapi-tests/testGetPropertyDefault.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsdate.cpp
js/src/jsnum.cpp
js/src/jsopcode.cpp
js/src/shell/js.cpp
js/src/xpconnect/shell/xpcshell.cpp
js/src/xpconnect/src/xpcwrappednativejsops.cpp
--- a/js/src/jsapi-tests/testGetPropertyDefault.cpp
+++ b/js/src/jsapi-tests/testGetPropertyDefault.cpp
@@ -5,21 +5,17 @@
 #include "tests.h"
 
 #define JSVAL_IS_FALSE(x) ((JSVAL_IS_BOOLEAN(x)) && !(JSVAL_TO_BOOLEAN(x)))
 #define JSVAL_IS_TRUE(x)  ((JSVAL_IS_BOOLEAN(x)) && (JSVAL_TO_BOOLEAN(x)))
 
 static JSBool
 stringToId(JSContext *cx, const char *s, jsid *idp)
 {
-    char *buf = JS_strdup(cx, s);
-    if (!buf)
-        return false;
-
-    JSString *str = JS_NewString(cx, buf, strlen(s));
+    JSString *str = JS_NewStringCopyZ(cx, s);
     if (!str)
         return false;
 
     return JS_ValueToId(cx, STRING_TO_JSVAL(str), idp);
 }
 
 BEGIN_TEST(testGetPropertyDefault_bug594060)
 {
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -5178,18 +5178,22 @@ JS_NewString(JSContext *cx, char *bytes,
 
     /* Make a UTF-16 vector from the 8-bit char codes in bytes. */
     chars = js_InflateString(cx, bytes, &length);
     if (!chars)
         return NULL;
 
     /* Free chars (but not bytes, which caller frees on error) if we fail. */
     str = js_NewString(cx, chars, length);
-    if (!str)
+    if (!str) {
         cx->free(chars);
+        return NULL;
+    }
+
+    js_free(bytes);
     return str;
 }
 
 JS_PUBLIC_API(JSString *)
 JS_NewStringCopyN(JSContext *cx, const char *s, size_t n)
 {
     jschar *js;
     JSString *str;
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3143,16 +3143,22 @@ class JSAutoByteString {
       : mBytes(NULL) {
         JS_GUARD_OBJECT_NOTIFIER_INIT;
     }
 
     ~JSAutoByteString() {
         js_free(mBytes);
     }
 
+    /* Take ownership of the given byte array. */
+    void initBytes(char *bytes) {
+        JS_ASSERT(!mBytes);
+        mBytes = bytes;
+    }
+
     char *encode(JSContext *cx, JSString *str) {
         JS_ASSERT(!mBytes);
         JS_ASSERT(cx);
         mBytes = JS_EncodeString(cx, str);
         return mBytes;
     }
 
     void clear() {
--- a/js/src/jsdate.cpp
+++ b/js/src/jsdate.cpp
@@ -2350,21 +2350,20 @@ date_toSource(JSContext *cx, uintN argc,
     }
 
     bytes = JS_smprintf("(new %s(%s))", js_Date_str, numStr);
     if (!bytes) {
         JS_ReportOutOfMemory(cx);
         return JS_FALSE;
     }
 
-    str = JS_NewString(cx, bytes, strlen(bytes));
-    if (!str) {
-        js_free(bytes);
+    str = JS_NewStringCopyZ(cx, bytes);
+    js_free(bytes);
+    if (!str)
         return JS_FALSE;
-    }
     vp->setString(str);
     return JS_TRUE;
 }
 #endif
 
 static JSBool
 date_toString(JSContext *cx, uintN argc, Value *vp)
 {
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -807,21 +807,20 @@ num_toLocaleString(JSContext *cx, uintN 
         strcpy(tmpDest, nint + 1);
     } else {
         strcpy(tmpDest, nint);
     }
 
     if (cx->localeCallbacks && cx->localeCallbacks->localeToUnicode)
         return cx->localeCallbacks->localeToUnicode(cx, buf, Jsvalify(vp));
 
-    str = JS_NewString(cx, buf, size);
-    if (!str) {
-        cx->free(buf);
+    str = js_NewStringCopyN(cx, buf, size);
+    cx->free(buf);
+    if (!str)
         return JS_FALSE;
-    }
 
     vp->setString(str);
     return JS_TRUE;
 }
 
 JSBool
 js_num_valueOf(JSContext *cx, uintN argc, Value *vp)
 {
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -291,52 +291,53 @@ static bool
 ToDisassemblySource(JSContext *cx, jsval v, JSAutoByteString *bytes)
 {
     if (!JSVAL_IS_PRIMITIVE(v)) {
         JSObject *obj = JSVAL_TO_OBJECT(v);
         Class *clasp = obj->getClass();
 
         if (clasp == &js_BlockClass) {
             char *source = JS_sprintf_append(NULL, "depth %d {", OBJ_BLOCK_DEPTH(cx, obj));
+            if (!source)
+                return false;
 
             Shape::Range r = obj->lastProperty()->all();
             while (!r.empty()) {
                 const Shape &shape = r.front();
                 JSAutoByteString bytes;
                 if (!js_AtomToPrintableString(cx, JSID_TO_ATOM(shape.id), &bytes))
-                    return NULL;
+                    return false;
 
                 r.popFront();
                 source = JS_sprintf_append(source, "%s: %d%s",
                                            bytes.ptr(), shape.shortid,
                                            !r.empty() ? ", " : "");
+                if (!source)
+                    return false;
             }
 
             source = JS_sprintf_append(source, "}");
             if (!source)
-                return NULL;
-
-            JSString *str = JS_NewString(cx, source, strlen(source));
-            if (!str)
-                return NULL;
-            return bytes->encode(cx, str);
+                return false;
+            bytes->initBytes(source);
+            return true;
         }
 
         if (clasp == &js_FunctionClass) {
             JSFunction *fun = GET_FUNCTION_PRIVATE(cx, obj);
             JSString *str = JS_DecompileFunction(cx, fun, JS_DONT_PRETTY_PRINT);
             if (!str)
-                return NULL;
+                return false;
             return bytes->encode(cx, str);
         }
 
         if (clasp == &js_RegExpClass) {
             AutoValueRooter tvr(cx);
             if (!js_regexp_toString(cx, obj, tvr.addr()))
-                return NULL;
+                return false;
             return bytes->encode(cx, JSVAL_TO_STRING(Jsvalify(tvr.value())));
         }
     }
 
     return !!js_ValueToPrintable(cx, Valueify(v), bytes, true);
 }
 
 JS_FRIEND_API(uintN)
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -1008,21 +1008,20 @@ Options(JSContext *cx, uintN argc, jsval
         }
     }
     if (!found)
         names = strdup("");
     if (!names) {
         JS_ReportOutOfMemory(cx);
         return JS_FALSE;
     }
-    str = JS_NewString(cx, names, strlen(names));
-    if (!str) {
-        free(names);
+    str = JS_NewStringCopyZ(cx, names);
+    free(names);
+    if (!str)
         return JS_FALSE;
-    }
     *vp = STRING_TO_JSVAL(str);
     return JS_TRUE;
 }
 
 static JSBool
 Load(JSContext *cx, uintN argc, jsval *vp)
 {
     uintN i;
@@ -1131,21 +1130,20 @@ ReadLine(JSContext *cx, uintN argc, jsva
     }
 
     buf = tmp;
 
     /*
      * Turn buf into a JSString. Note that buflength includes the trailing null
      * character.
      */
-    str = JS_NewString(cx, buf, sawNewline ? buflength - 1 : buflength);
-    if (!str) {
-        JS_free(cx, buf);
+    str = JS_NewStringCopyN(cx, buf, sawNewline ? buflength - 1 : buflength);
+    JS_free(cx, buf);
+    if (!str)
         return JS_FALSE;
-    }
 
     *vp = STRING_TO_JSVAL(str);
     return JS_TRUE;
 }
 
 static JSBool
 PutStr(JSContext *cx, uintN argc, jsval *vp)
 {
@@ -4138,22 +4136,20 @@ Snarf(JSContext *cx, uintN argc, jsval *
         fclose(file);
     }
     JS_free(cx, (void*)pathname);
     if (!ok) {
         JS_free(cx, buf);
         return ok;
     }
 
-    buf[len] = '\0';
-    str = JS_NewString(cx, buf, len);
-    if (!str) {
-        JS_free(cx, buf);
+    str = JS_NewStringCopyN(cx, buf, len);
+    JS_free(cx, buf);
+    if (!str)
         return JS_FALSE;
-    }
     *vp = STRING_TO_JSVAL(str);
     return JS_TRUE;
 }
 
 JSBool
 Wrap(JSContext *cx, uintN argc, jsval *vp)
 {
     jsval v = argc > 0 ? JS_ARGV(cx, vp)[0] : JSVAL_VOID;
--- a/js/src/xpconnect/shell/xpcshell.cpp
+++ b/js/src/xpconnect/shell/xpcshell.cpp
@@ -819,21 +819,20 @@ Options(JSContext *cx, uintN argc, jsval
         }
     }
     if (!found)
         names = strdup("");
     if (!names) {
         JS_ReportOutOfMemory(cx);
         return JS_FALSE;
     }
-    str = JS_NewString(cx, names, strlen(names));
-    if (!str) {
-        free(names);
+    str = JS_NewStringCopyZ(cx, names);
+    free(names);
+    if (!str)
         return JS_FALSE;
-    }
     JS_SET_RVAL(cx, vp, STRING_TO_JSVAL(str));
     return JS_TRUE;
 }
 
 static JSBool
 Parent(JSContext *cx, uintN argc, jsval *vp)
 {
     if (argc != 1) {
--- a/js/src/xpconnect/src/xpcwrappednativejsops.cpp
+++ b/js/src/xpconnect/src/xpcwrappednativejsops.cpp
@@ -86,23 +86,20 @@ ToStringGuts(XPCCallContext& ccx)
         sz = JS_smprintf("[xpconnect wrapped native prototype]");
 
     if(!sz)
     {
         JS_ReportOutOfMemory(ccx);
         return JS_FALSE;
     }
 
-    JSString* str = JS_NewString(ccx, sz, strlen(sz));
+    JSString* str = JS_NewStringCopyZ(ccx, sz);
+    JS_smprintf_free(sz);
     if(!str)
-    {
-        JS_smprintf_free(sz);
-        // JS_ReportOutOfMemory already reported by failed JS_NewString
         return JS_FALSE;
-    }
 
     ccx.SetRetVal(STRING_TO_JSVAL(str));
     return JS_TRUE;
 }
 
 /***************************************************************************/
 
 static JSBool
@@ -124,23 +121,20 @@ XPC_WN_Shared_ToString(JSContext *cx, ui
 #  define FMT_ADDR ""
 #  define FMT_STR(str)
 #  define PARAM_ADDR(w)
 #endif
         char *sz = JS_smprintf("[object %s" FMT_ADDR FMT_STR(" (native") FMT_ADDR FMT_STR(")") "]", si->GetJSClass()->name PARAM_ADDR(obj) PARAM_ADDR(xpc_GetJSPrivate(obj)));
         if(!sz)
             return JS_FALSE;
 
-        JSString* str = JS_NewString(cx, sz, strlen(sz));
+        JSString* str = JS_NewStringCopyZ(cx, sz);
+        JS_smprintf_free(sz);
         if(!str)
-        {
-            JS_smprintf_free(sz);
-
             return JS_FALSE;
-        }
 
         *vp = STRING_TO_JSVAL(str);
 
         return JS_TRUE;
     }
     
     XPCCallContext ccx(JS_CALLER, cx, obj);
     ccx.SetName(ccx.GetRuntime()->GetStringID(XPCJSRuntime::IDX_TO_STRING));