Comments addressed
http://codereview.chromium.org/7039037/diff/1/src/json-parser.cc File src/json-parser.cc (right): http://codereview.chromium.org/7039037/diff/1/src/json-parser.cc#newcode46 src/json-parser.cc:46: is_sequential_ascii_ = true; On 2011/05/24 07:28:34, Lasse Reichstein wrote:
That what is not the case? That strings can change, or that the code
is
interrupted?
In either case, it's not true. The MakeExternal api function can be called from a weak callback
triggered by
GC, so any time you do allocation (since this is Handle code), your
string
representation could potentially change. It probably doesn't happen, but I can't guarantee that it isn't a
security
problem if it does. I.e., you can only cache the string shape for the duration of a AssertNoAllocation guard. ALternatively, put a guard on the MakeExternal api function that
disallows it
during GC (which is a very reasonable thing to do anyway - GC
callbacks should
be restricted in what they can do, and they currently aren't).
Obviously if you allow preemption (any stack-guards), things can
change at that
time too, but you don't do that here.
OK, changed so that we reinitialize this variable every time we call scanjson (this has a minor performance implication). http://codereview.chromium.org/7039037/diff/8001/src/json-parser.cc#newcode347 src/json-parser.cc:347: number_ = StringToDouble(isolate()->unicode_cache(), On 2011/05/24 07:28:34, Lasse Reichstein wrote:
You only need an ASCII buffer (we have already established that it is
a number
literal, so all characters are ASCII).
Done. http://codereview.chromium.org/7039037/diff/8001/src/json-parser.cc#newcode348 src/json-parser.cc:348: *value, On 2011/05/24 07:28:34, Lasse Reichstein wrote:
res -> result.
Done. http://codereview.chromium.org/7039037/diff/8001/src/json-parser.cc#newcode354 src/json-parser.cc:354: On 2011/05/24 07:28:34, Lasse Reichstein wrote:
Just drop the last argument (i.e., let it default to 0.0). We know that the string isn't empty anyway.
Done. http://codereview.chromium.org/7039037/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
