Bug 724586 - Use modern tools to clean up obj_ToSource; r=Waldo

This patch replaces the fiddly strcat logic with StringBuilder usage and cleans
up the error handling control flow using RAII.

--HG--
extra : rebase_source : d42753a880a8d242627534cd5827075ea4a8966d
This commit is contained in:
Terrence Cole 2012-02-07 10:39:58 -08:00
Родитель 42a56fd3ae
Коммит 4e00a198c7
5 изменённых файлов: 56 добавлений и 152 удалений

Просмотреть файл

@ -1007,7 +1007,6 @@ JSContext::JSContext(JSRuntime *rt)
, stackIterAssertionEnabled(true)
#endif
{
PodZero(&sharpObjectMap);
PodZero(&link);
#ifdef JS_THREADSAFE
PodZero(&threadLinks);

Просмотреть файл

@ -81,6 +81,8 @@ struct JSSharpObjectMap {
jsrefcount depth;
uint32_t sharpgen;
JSHashTable *table;
JSSharpObjectMap() : depth(0), sharpgen(0), table(NULL) {}
};
namespace js {

Просмотреть файл

@ -454,15 +454,11 @@ js_TraceSharpMap(JSTracer *trc, JSSharpObjectMap *map)
static JSBool
obj_toSource(JSContext *cx, uintN argc, Value *vp)
{
JSBool ok;
jschar *ochars, *vsharp;
const jschar *idstrchars, *vchars;
size_t nchars, idstrlength, gsoplength, vlength, vsharplength, curlen;
const char *comma;
bool comma = false;
const jschar *vchars;
size_t vlength;
Value *val;
JSString *gsop[2];
JSString *valstr, *str;
JSLinearString *idstr;
JS_CHECK_RECURSION(cx, return JS_FALSE);
@ -477,7 +473,6 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
if (!obj)
return false;
jschar *chars;
JSIdArray *ida;
bool alreadySeen = false;
JSHashEntry *he = js_EnterSharpObject(cx, obj, &ida, &alreadySeen);
@ -497,22 +492,26 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
return true;
}
JS_ASSERT(!IS_SHARP(he));
ok = JS_TRUE;
if (alreadySeen)
MAKE_SHARP(he);
/* If outermost, allocate 4 + 1 for "({})" and the terminator. */
chars = (jschar *) cx->malloc_(((outermost ? 4 : 2) + 1) * sizeof(jschar));
nchars = 0;
if (!chars)
goto error;
if (outermost)
chars[nchars++] = '(';
/* Automatically call js_LeaveSharpObject when we leave this frame. */
class AutoLeaveSharpObject {
JSContext *cx;
JSIdArray *ida;
public:
AutoLeaveSharpObject(JSContext *cx, JSIdArray *ida) : cx(cx), ida(ida) {}
~AutoLeaveSharpObject() {
js_LeaveSharpObject(cx, &ida);
}
} autoLeaveSharpObject(cx, ida);
chars[nchars++] = '{';
comma = NULL;
StringBuffer buf(cx);
if (outermost && !buf.append('('))
return false;
if (!buf.append('{'))
return false;
/*
* We have four local roots for cooked and raw value GC safety. Hoist the
@ -521,28 +520,26 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
*/
val = localroot + 2;
for (jsint i = 0, length = ida->length; i < length; i++) {
for (jsint i = 0; i < ida->length; i++) {
/* Get strings for id and value and GC-root them via vp. */
jsid id = ida->vector[i];
JSLinearString *idstr;
JSObject *obj2;
JSProperty *prop;
ok = obj->lookupGeneric(cx, id, &obj2, &prop);
if (!ok)
goto error;
if (!obj->lookupGeneric(cx, id, &obj2, &prop))
return false;
/*
* Convert id to a value and then to a string. Decide early whether we
* prefer get/set or old getter/setter syntax.
*/
JSString *s = ToString(cx, IdToValue(id));
if (!s || !(idstr = s->ensureLinear(cx))) {
ok = JS_FALSE;
goto error;
}
if (!s || !(idstr = s->ensureLinear(cx)))
return false;
vp->setString(idstr); /* local root */
jsint valcnt = 0;
int valcnt = 0;
if (prop) {
bool doGet = true;
if (obj2->isNative()) {
@ -564,9 +561,8 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
if (doGet) {
valcnt = 1;
gsop[0] = NULL;
ok = obj->getGeneric(cx, id, &val[0]);
if (!ok)
goto error;
if (!obj->getGeneric(cx, id, &val[0]))
return false;
}
}
@ -574,25 +570,16 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
* If id is a string that's not an identifier, or if it's a negative
* integer, then it must be quoted.
*/
bool idIsLexicalIdentifier = IsIdentifier(idstr);
if (JSID_IS_ATOM(id)
? !idIsLexicalIdentifier
? !IsIdentifier(idstr)
: (!JSID_IS_INT(id) || JSID_TO_INT(id) < 0)) {
s = js_QuoteString(cx, idstr, jschar('\''));
if (!s || !(idstr = s->ensureLinear(cx))) {
ok = JS_FALSE;
goto error;
}
if (!s || !(idstr = s->ensureLinear(cx)))
return false;
vp->setString(idstr); /* local root */
}
idstrlength = idstr->length();
idstrchars = idstr->getChars(cx);
if (!idstrchars) {
ok = JS_FALSE;
goto error;
}
for (jsint j = 0; j < valcnt; j++) {
for (int j = 0; j < valcnt; j++) {
/*
* Censor an accessor descriptor getter or setter part if it's
* undefined.
@ -601,27 +588,15 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
continue;
/* Convert val[j] to its canonical source form. */
valstr = js_ValueToSource(cx, val[j]);
if (!valstr) {
ok = JS_FALSE;
goto error;
}
JSString *valstr = js_ValueToSource(cx, val[j]);
if (!valstr)
return false;
localroot[j].setString(valstr); /* local root */
vchars = valstr->getChars(cx);
if (!vchars) {
ok = JS_FALSE;
goto error;
}
if (!vchars)
return false;
vlength = valstr->length();
/*
* If val[j] is a non-sharp object, and we're not serializing an
* accessor (ECMA syntax can't accommodate sharpened accessors),
* consider sharpening it.
*/
vsharp = NULL;
vsharplength = 0;
/*
* Remove '(function ' from the beginning of valstr and ')' from the
* end so that we can put "get" in front of the function definition.
@ -657,100 +632,34 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
}
}
#define SAFE_ADD(n) \
JS_BEGIN_MACRO \
size_t n_ = (n); \
curlen += n_; \
if (curlen < n_) \
goto overflow; \
JS_END_MACRO
if (comma && !buf.append(", "))
return false;
comma = true;
curlen = nchars;
if (comma)
SAFE_ADD(2);
SAFE_ADD(idstrlength + 1);
if (gsop[j])
SAFE_ADD(gsop[j]->length() + 1);
SAFE_ADD(vsharplength);
SAFE_ADD(vlength);
/* Account for the trailing null. */
SAFE_ADD((outermost ? 2 : 1) + 1);
#undef SAFE_ADD
if (!buf.append(gsop[j]) || !buf.append(' '))
return false;
if (curlen > size_t(-1) / sizeof(jschar))
goto overflow;
if (!buf.append(idstr))
return false;
if (!buf.append(gsop[j] ? ' ' : ':'))
return false;
/* Allocate 1 + 1 at end for closing brace and terminating 0. */
chars = (jschar *) cx->realloc_((ochars = chars), curlen * sizeof(jschar));
if (!chars) {
chars = ochars;
goto overflow;
}
if (comma) {
chars[nchars++] = comma[0];
chars[nchars++] = comma[1];
}
comma = ", ";
if (gsop[j]) {
gsoplength = gsop[j]->length();
const jschar *gsopchars = gsop[j]->getChars(cx);
if (!gsopchars)
goto overflow;
js_strncpy(&chars[nchars], gsopchars, gsoplength);
nchars += gsoplength;
chars[nchars++] = ' ';
}
js_strncpy(&chars[nchars], idstrchars, idstrlength);
nchars += idstrlength;
/* Extraneous space after id here will be extracted later */
chars[nchars++] = gsop[j] ? ' ' : ':';
if (vsharplength) {
js_strncpy(&chars[nchars], vsharp, vsharplength);
nchars += vsharplength;
}
js_strncpy(&chars[nchars], vchars, vlength);
nchars += vlength;
if (vsharp)
cx->free_(vsharp);
if (!buf.append(vchars, vlength))
return false;
}
}
chars[nchars++] = '}';
if (outermost)
chars[nchars++] = ')';
chars[nchars] = 0;
error:
js_LeaveSharpObject(cx, &ida);
if (!ok) {
if (chars)
Foreground::free_(chars);
if (!buf.append('}'))
return false;
}
if (!chars) {
JS_ReportOutOfMemory(cx);
if (outermost && !buf.append(')'))
return false;
}
str = js_NewString(cx, chars, nchars);
if (!str) {
cx->free_(chars);
JSString *str = buf.finishString();
if (!str)
return false;
}
vp->setString(str);
return true;
overflow:
cx->free_(vsharp);
cx->free_(chars);
chars = NULL;
goto error;
}
#endif /* JS_HAS_TOSOURCE */

Просмотреть файл

@ -1555,13 +1555,9 @@ class ValueArray {
/* For manipulating JSContext::sharpObjectMap. */
#define SHARP_BIT ((jsatomid) 1)
#define BUSY_BIT ((jsatomid) 2)
#define SHARP_ID_SHIFT 2
#define IS_SHARP(he) (uintptr_t((he)->value) & SHARP_BIT)
#define MAKE_SHARP(he) ((he)->value = (void *) (uintptr_t((he)->value)|SHARP_BIT))
#define IS_BUSY(he) (uintptr_t((he)->value) & BUSY_BIT)
#define MAKE_BUSY(he) ((he)->value = (void *) (uintptr_t((he)->value)|BUSY_BIT))
#define CLEAR_BUSY(he) ((he)->value = (void *) (uintptr_t((he)->value)&~BUSY_BIT))
extern JSHashEntry *
js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alreadySeen);

Просмотреть файл

@ -209,11 +209,9 @@ StringBuffer::appendInflated(const char *cstr, size_t cstrlen)
size_t lengthBefore = length();
if (!cb.growByUninitialized(cstrlen))
return false;
#if DEBUG
size_t oldcstrlen = cstrlen;
bool ok =
#endif
InflateStringToBuffer(context(), cstr, cstrlen, begin() + lengthBefore, &cstrlen);
DebugOnly<size_t> oldcstrlen = cstrlen;
DebugOnly<bool> ok = InflateStringToBuffer(context(), cstr, cstrlen,
begin() + lengthBefore, &cstrlen);
JS_ASSERT(ok && oldcstrlen == cstrlen);
return true;
}