From 06311e984f955178455c13e490936d4a225a3944 Mon Sep 17 00:00:00 2001 From: Keyhan Vakil Date: Wed, 3 Aug 2022 19:18:38 -0700 Subject: [PATCH] src,buffer: remove unused chars_written parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This parameter was always being set to `nullptr` by its callers, either explicitly or implicitly via the default argument. It was also buggy, as in some cases it wouldn't be written to, potentially leaking stack memory (see the early returns in `StringBytes::WriteUCS2`). Remove it entirely. PR-URL: https://github.com/nodejs/node/pull/44092 Reviewed-By: Feng Yu Reviewed-By: Anna Henningsen Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen Reviewed-By: Darshan Sen --- src/api/encoding.cc | 2 +- src/node_buffer.cc | 16 ++++------------ src/string_bytes.cc | 33 ++++++++------------------------- src/string_bytes.h | 6 ++---- 4 files changed, 15 insertions(+), 42 deletions(-) diff --git a/src/api/encoding.cc b/src/api/encoding.cc index f64aeee15c3b34..68278ff7371d80 100644 --- a/src/api/encoding.cc +++ b/src/api/encoding.cc @@ -150,7 +150,7 @@ ssize_t DecodeWrite(Isolate* isolate, size_t buflen, Local val, enum encoding encoding) { - return StringBytes::Write(isolate, buf, buflen, val, encoding, nullptr); + return StringBytes::Write(isolate, buf, buflen, val, encoding); } } // namespace node diff --git a/src/node_buffer.cc b/src/node_buffer.cc index aec97f15e2c809..b2b3cfd9c8faa1 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -667,12 +667,8 @@ void Fill(const FunctionCallbackInfo& args) { // Write initial String to Buffer, then use that memory to copy remainder // of string. Correct the string length for cases like HEX where less than // the total string length is written. - str_length = StringBytes::Write(env->isolate(), - ts_obj_data + start, - fill_length, - str_obj, - enc, - nullptr); + str_length = StringBytes::Write( + env->isolate(), ts_obj_data + start, fill_length, str_obj, enc); } start_fill: @@ -731,12 +727,8 @@ void StringWrite(const FunctionCallbackInfo& args) { if (max_length == 0) return args.GetReturnValue().Set(0); - uint32_t written = StringBytes::Write(env->isolate(), - ts_obj_data + offset, - max_length, - str, - encoding, - nullptr); + uint32_t written = StringBytes::Write( + env->isolate(), ts_obj_data + offset, max_length, str, encoding); args.GetReturnValue().Set(written); } diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 5b530c85477310..2f512a844f193d 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -260,12 +260,8 @@ static size_t hex_decode(char* buf, return i; } -size_t StringBytes::WriteUCS2(Isolate* isolate, - char* buf, - size_t buflen, - Local str, - int flags, - size_t* chars_written) { +size_t StringBytes::WriteUCS2( + Isolate* isolate, char* buf, size_t buflen, Local str, int flags) { uint16_t* const dst = reinterpret_cast(buf); size_t max_chars = buflen / sizeof(*dst); @@ -277,7 +273,6 @@ size_t StringBytes::WriteUCS2(Isolate* isolate, size_t nchars; if (aligned_dst == dst) { nchars = str->Write(isolate, dst, 0, max_chars, flags); - *chars_written = nchars; return nchars * sizeof(*dst); } @@ -285,7 +280,9 @@ size_t StringBytes::WriteUCS2(Isolate* isolate, // Write all but the last char max_chars = std::min(max_chars, static_cast(str->Length())); - if (max_chars == 0) return 0; + if (max_chars == 0) { + return 0; + } nchars = str->Write(isolate, aligned_dst, 0, max_chars - 1, flags); CHECK_EQ(nchars, max_chars - 1); @@ -298,23 +295,16 @@ size_t StringBytes::WriteUCS2(Isolate* isolate, memcpy(buf + nchars * sizeof(*dst), &last, sizeof(last)); nchars++; - *chars_written = nchars; return nchars * sizeof(*dst); } - size_t StringBytes::Write(Isolate* isolate, char* buf, size_t buflen, Local val, - enum encoding encoding, - int* chars_written) { + enum encoding encoding) { HandleScope scope(isolate); size_t nbytes; - int nchars; - - if (chars_written == nullptr) - chars_written = &nchars; CHECK(val->IsString() == true); Local str = val.As(); @@ -334,19 +324,15 @@ size_t StringBytes::Write(Isolate* isolate, uint8_t* const dst = reinterpret_cast(buf); nbytes = str->WriteOneByte(isolate, dst, 0, buflen, flags); } - *chars_written = nbytes; break; case BUFFER: case UTF8: - nbytes = str->WriteUtf8(isolate, buf, buflen, chars_written, flags); + nbytes = str->WriteUtf8(isolate, buf, buflen, nullptr, flags); break; case UCS2: { - size_t nchars; - - nbytes = WriteUCS2(isolate, buf, buflen, str, flags, &nchars); - *chars_written = static_cast(nchars); + nbytes = WriteUCS2(isolate, buf, buflen, str, flags); // Node's "ucs2" encoding wants LE character data stored in // the Buffer, so we need to reorder on BE platforms. See @@ -368,7 +354,6 @@ size_t StringBytes::Write(Isolate* isolate, String::Value value(isolate, str); nbytes = base64_decode(buf, buflen, *value, value.length()); } - *chars_written = nbytes; break; case HEX: @@ -379,7 +364,6 @@ size_t StringBytes::Write(Isolate* isolate, String::Value value(isolate, str); nbytes = hex_decode(buf, buflen, *value, value.length()); } - *chars_written = nbytes; break; default: @@ -390,7 +374,6 @@ size_t StringBytes::Write(Isolate* isolate, return nbytes; } - // Quick and dirty size calculation // Will always be at least big enough, but may have some extra // UTF8 can be as much as 3x the size, Base64 can have 1-2 extra bytes diff --git a/src/string_bytes.h b/src/string_bytes.h index 69bb828e018cb0..ad1f15b05704c8 100644 --- a/src/string_bytes.h +++ b/src/string_bytes.h @@ -75,8 +75,7 @@ class StringBytes { char* buf, size_t buflen, v8::Local val, - enum encoding enc, - int* chars_written = nullptr); + enum encoding enc); // Take the bytes in the src, and turn it into a Buffer or String. static v8::MaybeLocal Encode(v8::Isolate* isolate, @@ -111,8 +110,7 @@ class StringBytes { char* buf, size_t buflen, v8::Local str, - int flags, - size_t* chars_written); + int flags); }; } // namespace node