Skip to content

Commit e38ce27

Browse files
authored
src: refactor WriteUCS2 and remove flags argument
This change refactors `StringBytes::WriteUCS2()` in multiple ways. The `flags` argument being passed to `WriteUCS2()` is not useful: the only really relevant flag is `NO_NULL_TERMINATION` since V8 ignores `REPLACE_INVALID_UTF8`, `HINT_MANY_WRITES_EXPECTED`, and `PRESERVE_ONE_BYTE_NULL` for UTF-16 strings. However, `WriteUCS2()` might not null-terminate the result correctly regardless of whether `NO_NULL_TERMINATION` is set because it makes multiple calls to `String::Write()` internally. For these reasons, this patch removes the `flags` argument entirely and always assumes `NO_NULL_TERMINATION`. Next, this patch replaces the calls to the deprecated function `String::Write()` with calls to the new function `String::WriteV2()`, which always succeeds and always writes a predictable number of characters, removing the need to deal with a return value here. Lastly, this patch simplifies the implementation of `WriteUCS2()` and computes the exact number of characters `nchars` from the beginning, removing the need to later check again if the number of characters is zero. PR-URL: #58163 Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent e5e8eaa commit e38ce27

File tree

2 files changed

+23
-31
lines changed

2 files changed

+23
-31
lines changed

src/string_bytes.cc

+22-29
Original file line numberDiff line numberDiff line change
@@ -203,40 +203,34 @@ static size_t keep_buflen_in_range(size_t len) {
203203
return len;
204204
}
205205

206-
size_t StringBytes::WriteUCS2(
207-
Isolate* isolate, char* buf, size_t buflen, Local<String> str, int flags) {
206+
size_t StringBytes::WriteUCS2(Isolate* isolate,
207+
char* buf,
208+
size_t buflen,
209+
Local<String> str) {
208210
uint16_t* const dst = reinterpret_cast<uint16_t*>(buf);
209211

210-
size_t max_chars = buflen / sizeof(*dst);
211-
if (max_chars == 0) {
212+
const size_t max_chars = buflen / sizeof(*dst);
213+
const size_t nchars = std::min(max_chars, static_cast<size_t>(str->Length()));
214+
if (nchars == 0) {
212215
return 0;
213216
}
214217

215218
uint16_t* const aligned_dst = nbytes::AlignUp(dst, sizeof(*dst));
216-
size_t nchars;
219+
CHECK_EQ(reinterpret_cast<uintptr_t>(aligned_dst) % sizeof(*dst), 0);
217220
if (aligned_dst == dst) {
218-
nchars = str->Write(isolate, dst, 0, max_chars, flags);
219-
return nchars * sizeof(*dst);
220-
}
221+
str->WriteV2(isolate, 0, nchars, dst);
222+
} else {
223+
// Write all but the last char.
224+
str->WriteV2(isolate, 0, nchars - 1, aligned_dst);
221225

222-
CHECK_EQ(reinterpret_cast<uintptr_t>(aligned_dst) % sizeof(*dst), 0);
226+
// Shift everything to unaligned-left.
227+
memmove(dst, aligned_dst, (nchars - 1) * sizeof(*dst));
223228

224-
// Write all but the last char
225-
max_chars = std::min(max_chars, static_cast<size_t>(str->Length()));
226-
if (max_chars == 0) {
227-
return 0;
229+
// One more char to be written.
230+
uint16_t last;
231+
str->WriteV2(isolate, nchars - 1, 1, &last);
232+
memcpy(dst + nchars - 1, &last, sizeof(last));
228233
}
229-
nchars = str->Write(isolate, aligned_dst, 0, max_chars - 1, flags);
230-
CHECK_EQ(nchars, max_chars - 1);
231-
232-
// Shift everything to unaligned-left
233-
memmove(dst, aligned_dst, nchars * sizeof(*dst));
234-
235-
// One more char to be written
236-
uint16_t last;
237-
CHECK_EQ(str->Write(isolate, &last, nchars, 1, flags), 1);
238-
memcpy(buf + nchars * sizeof(*dst), &last, sizeof(last));
239-
nchars++;
240234

241235
return nchars * sizeof(*dst);
242236
}
@@ -253,10 +247,6 @@ size_t StringBytes::Write(Isolate* isolate,
253247
Local<String> str = val.As<String>();
254248
String::ValueView input_view(isolate, str);
255249

256-
int flags = String::HINT_MANY_WRITES_EXPECTED |
257-
String::NO_NULL_TERMINATION |
258-
String::REPLACE_INVALID_UTF8;
259-
260250
switch (encoding) {
261251
case ASCII:
262252
case LATIN1:
@@ -265,6 +255,9 @@ size_t StringBytes::Write(Isolate* isolate,
265255
memcpy(buf, input_view.data8(), nbytes);
266256
} else {
267257
uint8_t* const dst = reinterpret_cast<uint8_t*>(buf);
258+
const int flags = String::HINT_MANY_WRITES_EXPECTED |
259+
String::NO_NULL_TERMINATION |
260+
String::REPLACE_INVALID_UTF8;
268261
nbytes = str->WriteOneByte(isolate, dst, 0, buflen, flags);
269262
}
270263
break;
@@ -276,7 +269,7 @@ size_t StringBytes::Write(Isolate* isolate,
276269
break;
277270

278271
case UCS2: {
279-
nbytes = WriteUCS2(isolate, buf, buflen, str, flags);
272+
nbytes = WriteUCS2(isolate, buf, buflen, str);
280273

281274
// Node's "ucs2" encoding wants LE character data stored in
282275
// the Buffer, so we need to reorder on BE platforms. See

src/string_bytes.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ class StringBytes {
9999
static size_t WriteUCS2(v8::Isolate* isolate,
100100
char* buf,
101101
size_t buflen,
102-
v8::Local<v8::String> str,
103-
int flags);
102+
v8::Local<v8::String> str);
104103
};
105104

106105
} // namespace node

0 commit comments

Comments
 (0)