this patch is almost ready to land.  There's one edge case which needs to be
handled, and some minor fixes for readability


https://codereview.chromium.org/121173009/diff/230001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/121173009/diff/230001/include/v8.h#newcode1650
include/v8.h:1650: DISALLOW_INVALID_UTF8 = 8
this needs a comment above

https://codereview.chromium.org/121173009/diff/230001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/121173009/diff/230001/src/api.cc#newcode4745
src/api.cc:4745: return writer.CompleteWrite(write_null, nchars_ref);
think you need to ensure that all return points from this function check
for the case that you  don't allow invalid utf8, and the last character
written is one half of a surrogate pair, which would be incorrectly
written as a replacement character in the current implementation

https://codereview.chromium.org/121173009/diff/230001/src/unicode.h
File src/unicode.h (right):

https://codereview.chromium.org/121173009/diff/230001/src/unicode.h#newcode51
src/unicode.h:51: * (e.g. an orphan surrogate) when converting to a UTF
encoding.
typo - UTF-8

https://codereview.chromium.org/121173009/diff/230001/src/unicode.h#newcode53
src/unicode.h:53: const int kReplacementCharacter = 0xFFFD;
this should be in Utf8, but see below

https://codereview.chromium.org/121173009/diff/230001/src/unicode.h#newcode161
src/unicode.h:161: bool allow_invalid);
this either needs to be an enum to avoid passing true and false
directly, or you need to keep the bool but use a default value of true
so existing call sites don't change

https://codereview.chromium.org/121173009/diff/230001/src/unicode.h#newcode165
src/unicode.h:165: static const uchar kBadChar = 0xFFFD;
hmmm, maybe you should just rename this variable here to
kReplacementChar, since that's what it is, and it might conflict with
your change.
you'll obviously need to check usages of it, but i think i removed most
of them in the codebase

https://codereview.chromium.org/121173009/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to