1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src, i18n: cleanup usage of MaybeStackBuffer

Browse Source 
- Templatize AsBuffer() and create a generic version for inclusion in
  the Buffer class
- Use MaybeStackBuffer::storage()
- If possible, avoid double conversion in ToASCII()/ToUnicode()
- More descriptive assertion error in tests

PR-URL: https://github.com/nodejs/node/pull/11464
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Timothy Gu 2017-02-19 20:04:10 -08:00
parent e7a3add531
commit 2cb5555f25
3 changed files with 85 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
src/node_i18n.cc
View File

@ -72,37 +72,19 @@ using v8::Value;
namespace i18n { namespace i18n {
const size_t kStorageSize = 1024; template <typename T>
MaybeLocal<Object> ToBufferEndian(Environment* env, MaybeStackBuffer<T>* buf) {
// TODO(jasnell): This could potentially become a member of MaybeStackBuffer MaybeLocal<Object> ret = Buffer::New(env, buf);
// at some point in the future. Care would need to be taken with the if (ret.IsEmpty())
// MaybeStackBuffer<UChar> variant below.
MaybeLocal<Object> AsBuffer(Isolate* isolate,
MaybeStackBuffer<char>* buf,
size_t len) {
if (buf->IsAllocated()) {
MaybeLocal<Object> ret = Buffer::New(isolate, buf->out(), len);
if (!ret.IsEmpty()) buf->Release();
return ret; return ret;
}
return Buffer::Copy(isolate, buf->out(), len);
}
MaybeLocal<Object> AsBuffer(Isolate* isolate, static_assert(sizeof(T) == 1 || sizeof(T) == 2,
MaybeStackBuffer<UChar>* buf, "Currently only one- or two-byte buffers are supported");
size_t len) { if (sizeof(T) > 1 && IsBigEndian()) {
char* dst = reinterpret_cast<char*>(**buf); SPREAD_BUFFER_ARG(ret.ToLocalChecked(), retbuf);
MaybeLocal<Object> ret; SwapBytes16(retbuf_data, retbuf_length);
if (buf->IsAllocated()) {
ret = Buffer::New(isolate, dst, len);
if (!ret.IsEmpty()) buf->Release();
} else {
ret = Buffer::Copy(isolate, dst, len);
}
if (!ret.IsEmpty() && IsBigEndian()) {
SPREAD_BUFFER_ARG(ret.ToLocalChecked(), buf);
SwapBytes16(buf_data, buf_length);
} }
return ret; return ret;
} }
@ -138,14 +120,14 @@ void CopySourceBuffer(MaybeStackBuffer<UChar>* dest,
} }
} }
typedef MaybeLocal<Object> (*TranscodeFunc)(Isolate* isolate, typedef MaybeLocal<Object> (*TranscodeFunc)(Environment* env,
const char* fromEncoding, const char* fromEncoding,
const char* toEncoding, const char* toEncoding,
const char* source, const char* source,
const size_t source_length, const size_t source_length,
UErrorCode* status); UErrorCode* status);
MaybeLocal<Object> Transcode(Isolate* isolate, MaybeLocal<Object> Transcode(Environment* env,
const char* fromEncoding, const char* fromEncoding,
const char* toEncoding, const char* toEncoding,
const char* source, const char* source,
@ -162,12 +144,14 @@ MaybeLocal<Object> Transcode(Isolate* isolate,
ucnv_convertEx(to.conv, from.conv, &target, target + limit, ucnv_convertEx(to.conv, from.conv, &target, target + limit,
&source, source + source_length, nullptr, nullptr, &source, source + source_length, nullptr, nullptr,
nullptr, nullptr, true, true, status); nullptr, nullptr, true, true, status);
if (U_SUCCESS(*status)) if (U_SUCCESS(*status)) {
ret = AsBuffer(isolate, &result, target - &result[0]); result.SetLength(target - &result[0]);
ret = ToBufferEndian(env, &result);
}
return ret; return ret;
} }
MaybeLocal<Object> TranscodeToUcs2(Isolate* isolate, MaybeLocal<Object> TranscodeToUcs2(Environment* env,
const char* fromEncoding, const char* fromEncoding,
const char* toEncoding, const char* toEncoding,
const char* source, const char* source,
@ -181,11 +165,11 @@ MaybeLocal<Object> TranscodeToUcs2(Isolate* isolate,
ucnv_toUChars(from.conv, *destbuf, length_in_chars, ucnv_toUChars(from.conv, *destbuf, length_in_chars,
source, source_length, status); source, source_length, status);
if (U_SUCCESS(*status)) if (U_SUCCESS(*status))
ret = AsBuffer(isolate, &destbuf, length_in_chars); ret = ToBufferEndian(env, &destbuf);
return ret; return ret;
} }
MaybeLocal<Object> TranscodeFromUcs2(Isolate* isolate, MaybeLocal<Object> TranscodeFromUcs2(Environment* env,
const char* fromEncoding, const char* fromEncoding,
const char* toEncoding, const char* toEncoding,
const char* source, const char* source,
@ -200,37 +184,42 @@ MaybeLocal<Object> TranscodeFromUcs2(Isolate* isolate,
MaybeStackBuffer<char> destbuf(length_in_chars); MaybeStackBuffer<char> destbuf(length_in_chars);
const uint32_t len = ucnv_fromUChars(to.conv, *destbuf, length_in_chars, const uint32_t len = ucnv_fromUChars(to.conv, *destbuf, length_in_chars,
*sourcebuf, length_in_chars, status); *sourcebuf, length_in_chars, status);
if (U_SUCCESS(*status)) if (U_SUCCESS(*status)) {
ret = AsBuffer(isolate, &destbuf, len); destbuf.SetLength(len);
ret = ToBufferEndian(env, &destbuf);
}
return ret; return ret;
} }
MaybeLocal<Object> TranscodeUcs2FromUtf8(Isolate* isolate, MaybeLocal<Object> TranscodeUcs2FromUtf8(Environment* env,
const char* fromEncoding, const char* fromEncoding,
const char* toEncoding, const char* toEncoding,
const char* source, const char* source,
const size_t source_length, const size_t source_length,
UErrorCode* status) { UErrorCode* status) {
*status = U_ZERO_ERROR; *status = U_ZERO_ERROR;
MaybeStackBuffer<UChar, kStorageSize> destbuf; MaybeStackBuffer<UChar> destbuf;
int32_t result_length; int32_t result_length;
u_strFromUTF8(*destbuf, kStorageSize, &result_length, u_strFromUTF8(*destbuf, destbuf.capacity(), &result_length,
source, source_length, status); source, source_length, status);
MaybeLocal<Object> ret; MaybeLocal<Object> ret;
if (U_SUCCESS(*status)) { if (U_SUCCESS(*status)) {
ret = AsBuffer(isolate, &destbuf, result_length * sizeof(**destbuf)); destbuf.SetLength(result_length);
ret = ToBufferEndian(env, &destbuf);
} else if (*status == U_BUFFER_OVERFLOW_ERROR) { } else if (*status == U_BUFFER_OVERFLOW_ERROR) {
*status = U_ZERO_ERROR; *status = U_ZERO_ERROR;
destbuf.AllocateSufficientStorage(result_length); destbuf.AllocateSufficientStorage(result_length);
u_strFromUTF8(*destbuf, result_length, &result_length, u_strFromUTF8(*destbuf, result_length, &result_length,
source, source_length, status); source, source_length, status);
if (U_SUCCESS(*status)) if (U_SUCCESS(*status)) {
ret = AsBuffer(isolate, &destbuf, result_length * sizeof(**destbuf)); destbuf.SetLength(result_length);
ret = ToBufferEndian(env, &destbuf);
}
} }
return ret; return ret;
} }
MaybeLocal<Object> TranscodeUtf8FromUcs2(Isolate* isolate, MaybeLocal<Object> TranscodeUtf8FromUcs2(Environment* env,
const char* fromEncoding, const char* fromEncoding,
const char* toEncoding, const char* toEncoding,
const char* source, const char* source,
@ -241,20 +230,21 @@ MaybeLocal<Object> TranscodeUtf8FromUcs2(Isolate* isolate,
const size_t length_in_chars = source_length / sizeof(UChar); const size_t length_in_chars = source_length / sizeof(UChar);
int32_t result_length; int32_t result_length;
MaybeStackBuffer<UChar> sourcebuf; MaybeStackBuffer<UChar> sourcebuf;
MaybeStackBuffer<char, kStorageSize> destbuf; MaybeStackBuffer<char> destbuf;
CopySourceBuffer(&sourcebuf, source, source_length, length_in_chars); CopySourceBuffer(&sourcebuf, source, source_length, length_in_chars);
u_strToUTF8(*destbuf, kStorageSize, &result_length, u_strToUTF8(*destbuf, destbuf.capacity(), &result_length,
*sourcebuf, length_in_chars, status); *sourcebuf, length_in_chars, status);
if (U_SUCCESS(*status)) { if (U_SUCCESS(*status)) {
ret = AsBuffer(isolate, &destbuf, result_length); destbuf.SetLength(result_length);
ret = ToBufferEndian(env, &destbuf);
} else if (*status == U_BUFFER_OVERFLOW_ERROR) { } else if (*status == U_BUFFER_OVERFLOW_ERROR) {
*status = U_ZERO_ERROR; *status = U_ZERO_ERROR;
destbuf.AllocateSufficientStorage(result_length); destbuf.AllocateSufficientStorage(result_length);
u_strToUTF8(*destbuf, result_length, &result_length, *sourcebuf, u_strToUTF8(*destbuf, result_length, &result_length, *sourcebuf,
length_in_chars, status); length_in_chars, status);
if (U_SUCCESS(*status)) { if (U_SUCCESS(*status)) {
ret = Buffer::New(isolate, *destbuf, result_length); destbuf.SetLength(result_length);
destbuf.Release(); ret = ToBufferEndian(env, &destbuf);
} }
} }
return ret; return ret;
@ -320,7 +310,7 @@ void Transcode(const FunctionCallbackInfo<Value>&args) {
ABORT(); ABORT();
} }
result = tfn(isolate, EncodingName(fromEncoding), EncodingName(toEncoding), result = tfn(env, EncodingName(fromEncoding), EncodingName(toEncoding),
ts_obj_data, ts_obj_length, &status); ts_obj_data, ts_obj_length, &status);
} else { } else {
status = U_ILLEGAL_ARGUMENT_ERROR; status = U_ILLEGAL_ARGUMENT_ERROR;
@ -431,7 +421,7 @@ int32_t ToUnicode(MaybeStackBuffer<char>* buf,
int32_t len = uidna_nameToUnicodeUTF8(uidna, int32_t len = uidna_nameToUnicodeUTF8(uidna,
input, length, input, length,
**buf, buf->length(), **buf, buf->capacity(),
&info, &info,
&status); &status);
@ -440,13 +430,17 @@ int32_t ToUnicode(MaybeStackBuffer<char>* buf,
buf->AllocateSufficientStorage(len); buf->AllocateSufficientStorage(len);
len = uidna_nameToUnicodeUTF8(uidna, len = uidna_nameToUnicodeUTF8(uidna,
input, length, input, length,
**buf, buf->length(), **buf, buf->capacity(),
&info, &info,
&status); &status);
} }
if (U_FAILURE(status)) if (U_FAILURE(status)) {
len = -1; len = -1;
buf->SetLength(0);
} else {
buf->SetLength(len);
}
uidna_close(uidna); uidna_close(uidna);
return len; return len;
@ -465,7 +459,7 @@ int32_t ToASCII(MaybeStackBuffer<char>* buf,
int32_t len = uidna_nameToASCII_UTF8(uidna, int32_t len = uidna_nameToASCII_UTF8(uidna,
input, length, input, length,
**buf, buf->length(), **buf, buf->capacity(),
&info, &info,
&status); &status);
@ -474,13 +468,17 @@ int32_t ToASCII(MaybeStackBuffer<char>* buf,
buf->AllocateSufficientStorage(len); buf->AllocateSufficientStorage(len);
len = uidna_nameToASCII_UTF8(uidna, len = uidna_nameToASCII_UTF8(uidna,
input, length, input, length,
**buf, buf->length(), **buf, buf->capacity(),
&info, &info,
&status); &status);
} }
if (U_FAILURE(status)) if (U_FAILURE(status)) {
len = -1; len = -1;
buf->SetLength(0);
} else {
buf->SetLength(len);
}
uidna_close(uidna); uidna_close(uidna);
return len; return len;

29
src/node_internals.h
View File

@ -204,6 +204,35 @@ v8::MaybeLocal<v8::Object> New(Environment* env,
// because ArrayBufferAllocator::Free() deallocates it again with free(). // because ArrayBufferAllocator::Free() deallocates it again with free().
// Mixing operator new and free() is undefined behavior so don't do that. // Mixing operator new and free() is undefined behavior so don't do that.
v8::MaybeLocal<v8::Object> New(Environment* env, char* data, size_t length); v8::MaybeLocal<v8::Object> New(Environment* env, char* data, size_t length);
// Construct a Buffer from a MaybeStackBuffer (and also its subclasses like
// Utf8Value and TwoByteValue).
// If |buf| is invalidated, an empty MaybeLocal is returned, and nothing is
// changed.
// If |buf| contains actual data, this method takes ownership of |buf|'s
// underlying buffer. However, |buf| itself can be reused even after this call,
// but its capacity, if increased through AllocateSufficientStorage, is not
// guaranteed to stay the same.
template <typename T>
static v8::MaybeLocal<v8::Object> New(Environment* env,
MaybeStackBuffer<T>* buf) {
v8::MaybeLocal<v8::Object> ret;
char* src = reinterpret_cast<char*>(buf->out());
const size_t len_in_bytes = buf->length() * sizeof(buf->out()[0]);
if (buf->IsAllocated())
ret = New(env, src, len_in_bytes);
else if (!buf->IsInvalidated())
ret = Copy(env, src, len_in_bytes);
if (ret.IsEmpty())
return ret;
if (buf->IsAllocated())
buf->Release();
return ret;
}
} // namespace Buffer } // namespace Buffer
} // namespace node } // namespace node

4
test/parallel/test-icu-transcode.js
View File

@ -22,9 +22,9 @@ const tests = {
for (const test in tests) { for (const test in tests) {
const dest = buffer.transcode(orig, 'utf8', test); const dest = buffer.transcode(orig, 'utf8', test);
assert.strictEqual(dest.length, tests[test].length); assert.strictEqual(dest.length, tests[test].length, `utf8->${test} length`);
for (let n = 0; n < tests[test].length; n++) for (let n = 0; n < tests[test].length; n++)
assert.strictEqual(dest[n], tests[test][n]); assert.strictEqual(dest[n], tests[test][n], `utf8->${test} char ${n}`);
} }
{ {