src,buffer: remove unused chars_written parameter

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: #44092 Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
nodejs · Aug 11, 2022 · 08f6f1c · 08f6f1c
1 parent e6a47e8
commit 08f6f1c
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 42 deletions.
diff --git a/src/api/encoding.cc b/src/api/encoding.cc
@@ -150,7 +150,7 @@ ssize_t DecodeWrite(Isolate* isolate,
                     size_t buflen,
                     Local<Value> 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
@@ -666,12 +666,8 @@ void Fill(const FunctionCallbackInfo<Value>& 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:
@@ -730,12 +726,8 @@ void StringWrite(const FunctionCallbackInfo<Value>& 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
@@ -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<String> str,
-                              int flags,
-                              size_t* chars_written) {
+size_t StringBytes::WriteUCS2(
+    Isolate* isolate, char* buf, size_t buflen, Local<String> str, int flags) {
   uint16_t* const dst = reinterpret_cast<uint16_t*>(buf);
 
   size_t max_chars = buflen / sizeof(*dst);
@@ -277,15 +273,16 @@ 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);
   }
 
   CHECK_EQ(reinterpret_cast<uintptr_t>(aligned_dst) % sizeof(*dst), 0);
 
   // Write all but the last char
   max_chars = std::min(max_chars, static_cast<size_t>(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<Value> 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<String> str = val.As<String>();
@@ -334,19 +324,15 @@ size_t StringBytes::Write(Isolate* isolate,
         uint8_t* const dst = reinterpret_cast<uint8_t*>(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<int>(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
@@ -75,8 +75,7 @@ class StringBytes {
                       char* buf,
                       size_t buflen,
                       v8::Local<v8::Value> 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<v8::Value> Encode(v8::Isolate* isolate,
@@ -111,8 +110,7 @@ class StringBytes {
                           char* buf,
                           size_t buflen,
                           v8::Local<v8::String> str,
-                          int flags,
-                          size_t* chars_written);
+                          int flags);
 };
 
 }  // namespace node