From 947564fc460e1c12c3f7f3125f746bd5867fb6b4 Mon Sep 17 00:00:00 2001 From: Tom Schuster Date: Tue, 6 Dec 2011 11:31:00 +0100 Subject: [PATCH] Bug 700169 - Refactor code to use StringBuffer. r=Waldo --HG-- extra : rebase_source : fefe9dcae3b2e227b9b32a17a976320eb9d71ba3 --- js/src/jit-test/tests/basic/error-toString.js | 9 + js/src/jsbool.cpp | 10 +- js/src/jsdate.cpp | 23 +- js/src/jsexn.cpp | 296 ++++++++---------- js/src/jsnum.cpp | 12 +- js/src/jsobj.cpp | 28 +- js/src/jsstr.cpp | 29 +- js/src/tests/js1_5/GC/regress-348532.js | 1 - 8 files changed, 162 insertions(+), 246 deletions(-) create mode 100644 js/src/jit-test/tests/basic/error-toString.js diff --git a/js/src/jit-test/tests/basic/error-toString.js b/js/src/jit-test/tests/basic/error-toString.js new file mode 100644 index 000000000000..d6709adb4700 --- /dev/null +++ b/js/src/jit-test/tests/basic/error-toString.js @@ -0,0 +1,9 @@ +var errorToString = Error.prototype.toString; + + +assertEq(errorToString.call({message: "", name: ""}), "Error"); +assertEq(errorToString.call({name: undefined, message: ""}), "Error"); +assertEq(errorToString.call({name: "Test", message: undefined}), "Test"); +assertEq(errorToString.call({name: "Test", message: ""}), "Test"); +assertEq(errorToString.call({name: "", message: "Test"}), "Test"); +assertEq(errorToString.call({name: "Test", message: "it!"}), "Test: it!"); diff --git a/js/src/jsbool.cpp b/js/src/jsbool.cpp index a28a9231eb7e..e3f15729f6d7 100644 --- a/js/src/jsbool.cpp +++ b/js/src/jsbool.cpp @@ -76,8 +76,6 @@ Class js::BooleanClass = { }; #if JS_HAS_TOSOURCE -#include "jsprf.h" - static JSBool bool_toSource(JSContext *cx, uintN argc, Value *vp) { @@ -87,9 +85,11 @@ bool_toSource(JSContext *cx, uintN argc, Value *vp) if (!BoxedPrimitiveMethodGuard(cx, args, bool_toSource, &b, &ok)) return ok; - char buf[32]; - JS_snprintf(buf, sizeof buf, "(new Boolean(%s))", JS_BOOLEAN_STR(b)); - JSString *str = JS_NewStringCopyZ(cx, buf); + StringBuffer sb(cx); + if (!sb.append("(new Boolean(") || !BooleanToStringBuffer(cx, b, sb) || !sb.append("))")) + return false; + + JSString *str = sb.finishString(); if (!str) return false; args.rval().setString(str); diff --git a/js/src/jsdate.cpp b/js/src/jsdate.cpp index ac30a69f3416..30f3dab2bd59 100644 --- a/js/src/jsdate.cpp +++ b/js/src/jsdate.cpp @@ -78,6 +78,7 @@ #include "jsinferinlines.h" #include "jsobjinlines.h" +#include "jsstrinlines.h" #include "vm/Stack-inl.h" @@ -2455,9 +2456,6 @@ date_toDateString(JSContext *cx, uintN argc, Value *vp) } #if JS_HAS_TOSOURCE -#include -#include "jsnum.h" - static JSBool date_toSource(JSContext *cx, uintN argc, Value *vp) { @@ -2468,23 +2466,14 @@ date_toSource(JSContext *cx, uintN argc, Value *vp) if (!obj) return ok; - double utctime = obj->getDateUTCTime().toNumber(); - - ToCStringBuf cbuf; - char *numStr = NumberToCString(cx, &cbuf, utctime); - if (!numStr) { - JS_ReportOutOfMemory(cx); + StringBuffer sb(cx); + if (!sb.append("(new Date(") || !NumberValueToStringBuffer(cx, obj->getDateUTCTime(), sb) || + !sb.append("))")) + { return false; } - char *bytes = JS_smprintf("(new %s(%s))", js_Date_str, numStr); - if (!bytes) { - JS_ReportOutOfMemory(cx); - return false; - } - - JSString *str = JS_NewStringCopyZ(cx, bytes); - cx->free_(bytes); + JSString *str = sb.finishString(); if (!str) return false; args.rval().setString(str); diff --git a/js/src/jsexn.cpp b/js/src/jsexn.cpp index 0bce1ae50f66..153f40b435c7 100644 --- a/js/src/jsexn.cpp +++ b/js/src/jsexn.cpp @@ -776,68 +776,79 @@ Exception(JSContext *cx, uintN argc, Value *vp) return true; } -/* - * Convert to string. - * - * This method only uses JavaScript-modifiable properties name, message. It - * is left to the host to check for private data and report filename and line - * number information along with this message. - */ +/* ES5 15.11.4.4 (NB: with subsequent errata). */ static JSBool exn_toString(JSContext *cx, uintN argc, Value *vp) { - jsval v; - JSString *name, *message, *result; - jschar *chars, *cp; - size_t name_length, message_length, length; + CallArgs args = CallArgsFromVp(argc, vp); - JSObject *obj = ToObject(cx, &vp[1]); - if (!obj) - return JS_FALSE; - if (!obj->getProperty(cx, cx->runtime->atomState.nameAtom, &v)) - return JS_FALSE; - name = JSVAL_IS_STRING(v) ? JSVAL_TO_STRING(v) : cx->runtime->emptyString; - vp->setString(name); - - if (!JS_GetProperty(cx, obj, js_message_str, &v)) - return JS_FALSE; - message = JSVAL_IS_STRING(v) ? JSVAL_TO_STRING(v) - : cx->runtime->emptyString; - - if (message->length() != 0) { - name_length = name->length(); - message_length = message->length(); - length = (name_length ? name_length + 2 : 0) + message_length; - cp = chars = (jschar *) cx->malloc_((length + 1) * sizeof(jschar)); - if (!chars) - return JS_FALSE; - - if (name_length) { - const jschar *name_chars = name->getChars(cx); - if (!name_chars) - return JS_FALSE; - js_strncpy(cp, name_chars, name_length); - cp += name_length; - *cp++ = ':'; *cp++ = ' '; - } - const jschar *message_chars = message->getChars(cx); - if (!message_chars) - return JS_FALSE; - js_strncpy(cp, message_chars, message_length); - cp += message_length; - *cp = 0; - - result = js_NewString(cx, chars, length); - if (!result) { - cx->free_(chars); - return JS_FALSE; - } - } else { - result = name; + /* Step 2. */ + if (!args.thisv().isObject()) { + JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_PROTOTYPE, "Error"); + return false; } - vp->setString(result); - return JS_TRUE; + /* Step 1. */ + JSObject &obj = args.thisv().toObject(); + + /* Step 3. */ + Value nameVal; + if (!obj.getProperty(cx, cx->runtime->atomState.nameAtom, &nameVal)) + return false; + + /* Step 4. */ + JSString *name; + if (nameVal.isUndefined()) { + name = CLASS_ATOM(cx, Error); + } else { + name = js_ValueToString(cx, nameVal); + if (!name) + return false; + } + + /* Step 5. */ + Value msgVal; + if (!obj.getProperty(cx, cx->runtime->atomState.messageAtom, &msgVal)) + return false; + + /* Step 6. */ + JSString *message; + if (msgVal.isUndefined()) { + message = cx->runtime->emptyString; + } else { + message = js_ValueToString(cx, msgVal); + if (!message) + return false; + } + + /* Step 7. */ + if (name->empty() && message->empty()) { + args.rval().setString(CLASS_ATOM(cx, Error)); + return true; + } + + /* Step 8. */ + if (name->empty()) { + args.rval().setString(message); + return true; + } + + /* Step 9. */ + if (message->empty()) { + args.rval().setString(name); + return true; + } + + /* Step 10. */ + StringBuffer sb(cx); + if (!sb.append(name) || !sb.append(": ") || !sb.append(message)) + return false; + + JSString *str = sb.finishString(); + if (!str) + return false; + args.rval().setString(str); + return true; } #if JS_HAS_TOSOURCE @@ -847,134 +858,75 @@ exn_toString(JSContext *cx, uintN argc, Value *vp) static JSBool exn_toSource(JSContext *cx, uintN argc, Value *vp) { - JSString *name, *message, *filename, *lineno_as_str, *result; - jsval localroots[3] = {JSVAL_NULL, JSVAL_NULL, JSVAL_NULL}; - size_t lineno_length, name_length, message_length, filename_length, length; - jschar *chars, *cp; + CallArgs args = CallArgsFromVp(argc, vp); JSObject *obj = ToObject(cx, &vp[1]); if (!obj) return false; + if (!obj->getProperty(cx, cx->runtime->atomState.nameAtom, vp)) return false; - name = js_ValueToString(cx, *vp); + + JSString *name = js_ValueToString(cx, *vp); if (!name) return false; vp->setString(name); + Value messageVal; + JSString *message; + if (!obj->getProperty(cx, cx->runtime->atomState.messageAtom, &messageVal) || + !(message = js_ValueToSource(cx, messageVal))) { - AutoArrayRooter tvr(cx, ArrayLength(localroots), localroots); - -#ifdef __GNUC__ - message = filename = NULL; -#endif - if (!JS_GetProperty(cx, obj, js_message_str, &localroots[0]) || - !(message = js_ValueToSource(cx, localroots[0]))) { - return false; - } - localroots[0] = STRING_TO_JSVAL(message); - - if (!JS_GetProperty(cx, obj, js_fileName_str, &localroots[1]) || - !(filename = js_ValueToSource(cx, localroots[1]))) { - return false; - } - localroots[1] = STRING_TO_JSVAL(filename); - - if (!JS_GetProperty(cx, obj, js_lineNumber_str, &localroots[2])) - return false; - uint32_t lineno; - if (!ValueToECMAUint32(cx, localroots[2], &lineno)) - return false; - - if (lineno != 0) { - lineno_as_str = js_ValueToString(cx, localroots[2]); - if (!lineno_as_str) - return false; - lineno_length = lineno_as_str->length(); - } else { - lineno_as_str = NULL; - lineno_length = 0; - } - - /* Magic 8, for the characters in ``(new ())''. */ - name_length = name->length(); - message_length = message->length(); - length = 8 + name_length + message_length; - - filename_length = filename->length(); - if (filename_length != 0) { - /* append filename as ``, {filename}'' */ - length += 2 + filename_length; - if (lineno_as_str) { - /* append lineno as ``, {lineno_as_str}'' */ - length += 2 + lineno_length; - } - } else { - if (lineno_as_str) { - /* - * no filename, but have line number, - * need to append ``, "", {lineno_as_str}'' - */ - length += 6 + lineno_length; - } - } - - cp = chars = (jschar *) cx->malloc_((length + 1) * sizeof(jschar)); - if (!chars) - return false; - - *cp++ = '('; *cp++ = 'n'; *cp++ = 'e'; *cp++ = 'w'; *cp++ = ' '; - const jschar *name_chars = name->getChars(cx); - if (!name_chars) - return false; - js_strncpy(cp, name_chars, name_length); - cp += name_length; - *cp++ = '('; - const jschar *message_chars = message->getChars(cx); - if (!message_chars) - return false; - if (message_length != 0) { - js_strncpy(cp, message_chars, message_length); - cp += message_length; - } - - if (filename_length != 0) { - /* append filename as ``, {filename}'' */ - *cp++ = ','; *cp++ = ' '; - const jschar *filename_chars = filename->getChars(cx); - if (!filename_chars) - return false; - js_strncpy(cp, filename_chars, filename_length); - cp += filename_length; - } else { - if (lineno_as_str) { - /* - * no filename, but have line number, - * need to append ``, "", {lineno_as_str}'' - */ - *cp++ = ','; *cp++ = ' '; *cp++ = '"'; *cp++ = '"'; - } - } - if (lineno_as_str) { - /* append lineno as ``, {lineno_as_str}'' */ - *cp++ = ','; *cp++ = ' '; - const jschar *lineno_chars = lineno_as_str->getChars(cx); - if (!lineno_chars) - return false; - js_strncpy(cp, lineno_chars, lineno_length); - cp += lineno_length; - } - - *cp++ = ')'; *cp++ = ')'; *cp = 0; - - result = js_NewString(cx, chars, length); - if (!result) { - cx->free_(chars); - return false; - } - vp->setString(result); - return true; + return false; } + + Value filenameVal; + JSString *filename; + if (!obj->getProperty(cx, cx->runtime->atomState.fileNameAtom, &filenameVal) || + !(filename = js_ValueToSource(cx, filenameVal))) + { + return false; + } + + Value linenoVal; + uint32_t lineno; + if (!obj->getProperty(cx, cx->runtime->atomState.lineNumberAtom, &linenoVal) || + !ValueToECMAUint32(cx, linenoVal, &lineno)) + { + return false; + } + + StringBuffer sb(cx); + if (!sb.append("(new ") || !sb.append(name) || !sb.append("(")) + return false; + + if (!sb.append(message)) + return false; + + if (!filename->empty()) { + if (!sb.append(", ") || !sb.append(filename)) + return false; + } + if (lineno != 0) { + /* We have a line, but no filename, add empty string */ + if (filename->empty() && !sb.append(", \"\"")) + return false; + + JSString *linenumber = js_ValueToString(cx, linenoVal); + if (!linenumber) + return false; + if (!sb.append(", ") || !sb.append(linenumber)) + return false; + } + + if (!sb.append("))")) + return false; + + JSString *str = sb.finishString(); + if (!str) + return false; + vp->setString(str); + return true; } #endif diff --git a/js/src/jsnum.cpp b/js/src/jsnum.cpp index 75670c77cb2b..f6766b88ae6e 100644 --- a/js/src/jsnum.cpp +++ b/js/src/jsnum.cpp @@ -533,16 +533,14 @@ num_toSource(JSContext *cx, uintN argc, Value *vp) if (!BoxedPrimitiveMethodGuard(cx, args, num_toSource, &d, &ok)) return ok; - ToCStringBuf cbuf; - char *numStr = NumberToCString(cx, &cbuf, d); - if (!numStr) { - JS_ReportOutOfMemory(cx); + StringBuffer sb(cx); + if (!sb.append("(new Number(") || !NumberValueToStringBuffer(cx, NumberValue(d), sb) || + !sb.append("))")) + { return false; } - char buf[64]; - JS_snprintf(buf, sizeof buf, "(new %s(%s))", NumberClass.name, numStr); - JSString *str = js_NewStringCopyZ(cx, buf); + JSString *str = sb.finishString(); if (!str) return false; args.rval().setString(str); diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index ea7e306bf619..3bfbadf64fce 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -85,9 +85,10 @@ #include "jsarrayinlines.h" #include "jsinterpinlines.h" +#include "jsobjinlines.h" #include "jsscopeinlines.h" #include "jsscriptinlines.h" -#include "jsobjinlines.h" +#include "jsstrinlines.h" #include "vm/BooleanObject-inl.h" #include "vm/NumberObject-inl.h" @@ -843,25 +844,14 @@ obj_toStringHelper(JSContext *cx, JSObject *obj) if (obj->isProxy()) return Proxy::obj_toString(cx, obj); - const char *clazz = obj->getClass()->name; - size_t nchars = 9 + strlen(clazz); /* 9 for "[object ]" */ - jschar *chars = (jschar *) cx->malloc_((nchars + 1) * sizeof(jschar)); - if (!chars) + StringBuffer sb(cx); + const char *className = obj->getClass()->name; + if (!sb.append("[object ") || !sb.appendInflated(className, strlen(className)) || + !sb.append("]")) + { return NULL; - - const char *prefix = "[object "; - nchars = 0; - while ((chars[nchars] = (jschar)*prefix) != 0) - nchars++, prefix++; - while ((chars[nchars] = (jschar)*clazz) != 0) - nchars++, clazz++; - chars[nchars++] = ']'; - chars[nchars] = 0; - - JSString *str = js_NewString(cx, chars, nchars); - if (!str) - cx->free_(chars); - return str; + } + return sb.finishString(); } JSObject * diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp index a3075757688d..ea22e5290491 100644 --- a/js/src/jsstr.cpp +++ b/js/src/jsstr.cpp @@ -470,34 +470,13 @@ str_toSource(JSContext *cx, uintN argc, Value *vp) if (!str) return false; - char buf[16]; - size_t j = JS_snprintf(buf, sizeof buf, "(new String("); - - JS::Anchor anchor(str); - size_t k = str->length(); - const jschar *s = str->getChars(cx); - if (!s) + StringBuffer sb(cx); + if (!sb.append("(new String(") || !sb.append(str) || !sb.append("))")) return false; - size_t n = j + k + 2; - jschar *t = (jschar *) cx->malloc_((n + 1) * sizeof(jschar)); - if (!t) + str = sb.finishString(); + if (!str) return false; - - size_t i; - for (i = 0; i < j; i++) - t[i] = buf[i]; - for (j = 0; j < k; i++, j++) - t[i] = s[j]; - t[i++] = ')'; - t[i++] = ')'; - t[i] = 0; - - str = js_NewString(cx, t, n); - if (!str) { - cx->free_(t); - return false; - } args.rval().setString(str); return true; } diff --git a/js/src/tests/js1_5/GC/regress-348532.js b/js/src/tests/js1_5/GC/regress-348532.js index acb8948c3395..25d484bac4f3 100644 --- a/js/src/tests/js1_5/GC/regress-348532.js +++ b/js/src/tests/js1_5/GC/regress-348532.js @@ -54,7 +54,6 @@ function test() expectExitCode(0); expectExitCode(3); - actual = 0; // construct string of 1<<23 characters