src: use ArrayBufferViewContents more frequently

Using `ArrayBufferViewContents` over
`Buffer::Data()`/`Buffer::Length()` or `SPREAD_BUFFER_ARG` has the
advantages of creating fewer individual variables to keep track off,
not being a “magic” macro that creates variables, reducing code size,
and being faster when receiving on-heap TypedArrays.

PR-URL: https://github.com/nodejs/node/pull/27920
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

9 files changed, 77 insertions, 81 deletions

diff --git a/src/js_stream.cc b/src/js_stream.cc
index 1d61605d64..2663106ba7 100644
--- a/src/js_stream.cc
+++ b/src/js_stream.cc
@@ -167,9 +167,9 @@ void JSStream::ReadBuffer(const FunctionCallbackInfo<Value>& args) {
   JSStream* wrap;
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
 
-  CHECK(Buffer::HasInstance(args[0]));
-  char* data = Buffer::Data(args[0]);
-  int len = Buffer::Length(args[0]);
+  ArrayBufferViewContents<char> buffer(args[0]);
+  const char* data = buffer.data();
+  int len = buffer.length();
 
   // Repeatedly ask the stream's owner for memory, copy the data that we
   // just read from JS into those buffers and emit them as reads.
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 3b4be5a810..e6a88f6498 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -463,17 +463,17 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = env->isolate();
 
   THROW_AND_RETURN_UNLESS_BUFFER(env, args.This());
-  SPREAD_BUFFER_ARG(args.This(), ts_obj);
+  ArrayBufferViewContents<char> buffer(args.This());
 
-  if (ts_obj_length == 0)
+  if (buffer.length() == 0)
     return args.GetReturnValue().SetEmptyString();
 
-  SLICE_START_END(env, args[0], args[1], ts_obj_length)
+  SLICE_START_END(env, args[0], args[1], buffer.length())
 
   Local<Value> error;
   MaybeLocal<Value> ret =
       StringBytes::Encode(isolate,
-                          ts_obj_data + start,
+                          buffer.data() + start,
                           length,
                           encoding,
                           &error);
@@ -492,9 +492,8 @@ void Copy(const FunctionCallbackInfo<Value> &args) {
 
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
-  Local<Object> buffer_obj = args[0].As<Object>();
+  ArrayBufferViewContents<char> source(args[0]);
   Local<Object> target_obj = args[1].As<Object>();
-  SPREAD_BUFFER_ARG(buffer_obj, ts_obj);
   SPREAD_BUFFER_ARG(target_obj, target);
 
   size_t target_start = 0;
@@ -503,14 +502,14 @@ void Copy(const FunctionCallbackInfo<Value> &args) {
 
   THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &target_start));
   THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &source_start));
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], ts_obj_length,
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], source.length(),
                                           &source_end));
 
   // Copy 0 bytes; we're done
   if (target_start >= target_length || source_start >= source_end)
     return args.GetReturnValue().Set(0);
 
-  if (source_start > ts_obj_length)
+  if (source_start > source.length())
     return THROW_ERR_OUT_OF_RANGE(
         env, "The value of \"sourceStart\" is out of range.");
 
@@ -519,9 +518,9 @@ void Copy(const FunctionCallbackInfo<Value> &args) {
 
   uint32_t to_copy = std::min(
       std::min(source_end - source_start, target_length - target_start),
-      ts_obj_length - source_start);
+      source.length() - source_start);
 
-  memmove(target_data + target_start, ts_obj_data + source_start, to_copy);
+  memmove(target_data + target_start, source.data() + source_start, to_copy);
   args.GetReturnValue().Set(to_copy);
 }
 
@@ -689,8 +688,8 @@ void CompareOffset(const FunctionCallbackInfo<Value> &args) {
 
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
-  SPREAD_BUFFER_ARG(args[0], ts_obj);
-  SPREAD_BUFFER_ARG(args[1], target);
+  ArrayBufferViewContents<char> source(args[0]);
+  ArrayBufferViewContents<char> target(args[1]);
 
   size_t target_start = 0;
   size_t source_start = 0;
@@ -699,15 +698,15 @@ void CompareOffset(const FunctionCallbackInfo<Value> &args) {
 
   THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &target_start));
   THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &source_start));
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], target_length,
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], target.length(),
                                           &target_end));
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[5], ts_obj_length,
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[5], source.length(),
                                           &source_end));
 
-  if (source_start > ts_obj_length)
+  if (source_start > source.length())
     return THROW_ERR_OUT_OF_RANGE(
         env, "The value of \"sourceStart\" is out of range.");
-  if (target_start > target_length)
+  if (target_start > target.length())
     return THROW_ERR_OUT_OF_RANGE(
         env, "The value of \"targetStart\" is out of range.");
 
@@ -716,11 +715,11 @@ void CompareOffset(const FunctionCallbackInfo<Value> &args) {
 
   size_t to_cmp =
       std::min(std::min(source_end - source_start, target_end - target_start),
-               ts_obj_length - source_start);
+               source.length() - source_start);
 
   int val = normalizeCompareVal(to_cmp > 0 ?
-                                  memcmp(ts_obj_data + source_start,
-                                         target_data + target_start,
+                                  memcmp(source.data() + source_start,
+                                         target.data() + target_start,
                                          to_cmp) : 0,
                                 source_end - source_start,
                                 target_end - target_start);
@@ -733,14 +732,14 @@ void Compare(const FunctionCallbackInfo<Value> &args) {
 
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
-  SPREAD_BUFFER_ARG(args[0], obj_a);
-  SPREAD_BUFFER_ARG(args[1], obj_b);
+  ArrayBufferViewContents<char> a(args[0]);
+  ArrayBufferViewContents<char> b(args[1]);
 
-  size_t cmp_length = std::min(obj_a_length, obj_b_length);
+  size_t cmp_length = std::min(a.length(), b.length());
 
   int val = normalizeCompareVal(cmp_length > 0 ?
-                                memcmp(obj_a_data, obj_b_data, cmp_length) : 0,
-                                obj_a_length, obj_b_length);
+                                memcmp(a.data(), b.data(), cmp_length) : 0,
+                                a.length(), b.length());
   args.GetReturnValue().Set(val);
 }
 
@@ -792,16 +791,16 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
   enum encoding enc = ParseEncoding(isolate, args[3], UTF8);
 
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
-  SPREAD_BUFFER_ARG(args[0], ts_obj);
+  ArrayBufferViewContents<char> buffer(args[0]);
 
   Local<String> needle = args[1].As<String>();
   int64_t offset_i64 = args[2].As<Integer>()->Value();
   bool is_forward = args[4]->IsTrue();
 
-  const char* haystack = ts_obj_data;
+  const char* haystack = buffer.data();
   // Round down to the nearest multiple of 2 in case of UCS2.
   const size_t haystack_length = (enc == UCS2) ?
-      ts_obj_length &~ 1 : ts_obj_length;  // NOLINT(whitespace/operators)
+      buffer.length() &~ 1 : buffer.length();  // NOLINT(whitespace/operators)
 
   size_t needle_length;
   if (!StringBytes::Size(isolate, needle, enc).To(&needle_length)) return;
@@ -909,15 +908,15 @@ void IndexOfBuffer(const FunctionCallbackInfo<Value>& args) {
 
   THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[0]);
   THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[1]);
-  SPREAD_BUFFER_ARG(args[0], ts_obj);
-  SPREAD_BUFFER_ARG(args[1], buf);
+  ArrayBufferViewContents<char> haystack_contents(args[0]);
+  ArrayBufferViewContents<char> needle_contents(args[1]);
   int64_t offset_i64 = args[2].As<Integer>()->Value();
   bool is_forward = args[4]->IsTrue();
 
-  const char* haystack = ts_obj_data;
-  const size_t haystack_length = ts_obj_length;
-  const char* needle = buf_data;
-  const size_t needle_length = buf_length;
+  const char* haystack = haystack_contents.data();
+  const size_t haystack_length = haystack_contents.length();
+  const char* needle = needle_contents.data();
+  const size_t needle_length = needle_contents.length();
 
   int64_t opt_offset = IndexOfOffset(haystack_length,
                                      offset_i64,
@@ -978,27 +977,28 @@ void IndexOfNumber(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[3]->IsBoolean());
 
   THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[0]);
-  SPREAD_BUFFER_ARG(args[0], ts_obj);
+  ArrayBufferViewContents<char> buffer(args[0]);
 
   uint32_t needle = args[1].As<Uint32>()->Value();
   int64_t offset_i64 = args[2].As<Integer>()->Value();
   bool is_forward = args[3]->IsTrue();
 
-  int64_t opt_offset = IndexOfOffset(ts_obj_length, offset_i64, 1, is_forward);
-  if (opt_offset <= -1 || ts_obj_length == 0) {
+  int64_t opt_offset =
+      IndexOfOffset(buffer.length(), offset_i64, 1, is_forward);
+  if (opt_offset <= -1 || buffer.length() == 0) {
     return args.GetReturnValue().Set(-1);
   }
   size_t offset = static_cast<size_t>(opt_offset);
-  CHECK_LT(offset, ts_obj_length);
+  CHECK_LT(offset, buffer.length());
 
   const void* ptr;
   if (is_forward) {
-    ptr = memchr(ts_obj_data + offset, needle, ts_obj_length - offset);
+    ptr = memchr(buffer.data() + offset, needle, buffer.length() - offset);
   } else {
-    ptr = node::stringsearch::MemrchrFill(ts_obj_data, needle, offset + 1);
+    ptr = node::stringsearch::MemrchrFill(buffer.data(), needle, offset + 1);
   }
   const char* ptr_char = static_cast<const char*>(ptr);
-  args.GetReturnValue().Set(ptr ? static_cast<int>(ptr_char - ts_obj_data)
+  args.GetReturnValue().Set(ptr ? static_cast<int>(ptr_char - buffer.data())
                                 : -1);
 }
 
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 76a8cc8a19..a5710dc33b 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -5766,11 +5766,10 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
   ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());
 
   THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Private key");
+  ArrayBufferViewContents<unsigned char> priv_buffer(args[0]);
 
   BignumPointer priv(BN_bin2bn(
-      reinterpret_cast<unsigned char*>(Buffer::Data(args[0].As<Object>())),
-      Buffer::Length(args[0].As<Object>()),
-      nullptr));
+      priv_buffer.data(), priv_buffer.length(), nullptr));
   if (!priv)
     return env->ThrowError("Failed to convert Buffer to BN");
 
@@ -6687,14 +6686,11 @@ OpenSSLBuffer ExportChallenge(const char* data, int len) {
 void ExportChallenge(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  size_t len = Buffer::Length(args[0]);
-  if (len == 0)
+  ArrayBufferViewContents<char> input(args[0]);
+  if (input.length() == 0)
     return args.GetReturnValue().SetEmptyString();
 
-  char* data = Buffer::Data(args[0]);
-  CHECK_NOT_NULL(data);
-
-  OpenSSLBuffer cert = ExportChallenge(data, len);
+  OpenSSLBuffer cert = ExportChallenge(input.data(), input.length());
   if (!cert)
     return args.GetReturnValue().SetEmptyString();
 
@@ -6749,16 +6745,13 @@ void ConvertKey(const FunctionCallbackInfo<Value>& args) {
 
 
 void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
-  CHECK(Buffer::HasInstance(args[0]));
-  CHECK(Buffer::HasInstance(args[1]));
-
-  size_t buf_length = Buffer::Length(args[0]);
-  CHECK_EQ(buf_length, Buffer::Length(args[1]));
+  ArrayBufferViewContents<char> buf1(args[0]);
+  ArrayBufferViewContents<char> buf2(args[1]);
 
-  const char* buf1 = Buffer::Data(args[0]);
-  const char* buf2 = Buffer::Data(args[1]);
+  CHECK_EQ(buf1.length(), buf2.length());
 
-  return args.GetReturnValue().Set(CRYPTO_memcmp(buf1, buf2, buf_length) == 0);
+  return args.GetReturnValue().Set(
+      CRYPTO_memcmp(buf1.data(), buf2.data(), buf1.length()) == 0);
 }
 
 void InitCryptoOnce() {
diff --git a/src/node_http_parser_impl.h b/src/node_http_parser_impl.h
index a354c6fcc5..1afedb0509 100644
--- a/src/node_http_parser_impl.h
+++ b/src/node_http_parser_impl.h
@@ -466,18 +466,15 @@ class Parser : public AsyncWrap, public StreamListener {
     CHECK(parser->current_buffer_.IsEmpty());
     CHECK_EQ(parser->current_buffer_len_, 0);
     CHECK_NULL(parser->current_buffer_data_);
-    CHECK_EQ(Buffer::HasInstance(args[0]), true);
 
-    Local<Object> buffer_obj = args[0].As<Object>();
-    char* buffer_data = Buffer::Data(buffer_obj);
-    size_t buffer_len = Buffer::Length(buffer_obj);
+    ArrayBufferViewContents<char> buffer(args[0]);
 
     // This is a hack to get the current_buffer to the callbacks with the least
     // amount of overhead. Nothing else will run while http_parser_execute()
     // runs, therefore this pointer can be set and used for the execution.
-    parser->current_buffer_ = buffer_obj;
+    parser->current_buffer_ = args[0].As<Object>();
 
-    Local<Value> ret = parser->Execute(buffer_data, buffer_len);
+    Local<Value> ret = parser->Execute(buffer.data(), buffer.length());
 
     if (!ret.IsEmpty())
       args.GetReturnValue().Set(ret);
@@ -643,7 +640,7 @@ class Parser : public AsyncWrap, public StreamListener {
   }
 
 
-  Local<Value> Execute(char* data, size_t len) {
+  Local<Value> Execute(const char* data, size_t len) {
     EscapableHandleScope scope(env()->isolate());
 
     current_buffer_len_ = len;
@@ -857,7 +854,7 @@ class Parser : public AsyncWrap, public StreamListener {
   bool got_exception_;
   Local<Object> current_buffer_;
   size_t current_buffer_len_;
-  char* current_buffer_data_;
+  const char* current_buffer_data_;
 #ifdef NODE_EXPERIMENTAL_HTTP
   unsigned int execute_depth_ = 0;
   bool pending_pause_ = false;
diff --git a/src/node_i18n.cc b/src/node_i18n.cc
index 8ccda35056..ad5c828392 100644
--- a/src/node_i18n.cc
+++ b/src/node_i18n.cc
@@ -205,14 +205,13 @@ class ConverterObject : public BaseObject, Converter {
 
     ConverterObject* converter;
     ASSIGN_OR_RETURN_UNWRAP(&converter, args[0].As<Object>());
-    SPREAD_BUFFER_ARG(args[1], input_obj);
+    ArrayBufferViewContents<char> input(args[1]);
     int flags = args[2]->Uint32Value(env->context()).ToChecked();
 
     UErrorCode status = U_ZERO_ERROR;
     MaybeStackBuffer<UChar> result;
     MaybeLocal<Object> ret;
-    size_t limit = ucnv_getMinCharSize(converter->conv) *
-                   input_obj_length;
+    size_t limit = ucnv_getMinCharSize(converter->conv) * input.length();
     if (limit > 0)
       result.AllocateSufficientStorage(limit);
 
@@ -225,8 +224,8 @@ class ConverterObject : public BaseObject, Converter {
       }
     });
 
-    const char* source = input_obj_data;
-    size_t source_length = input_obj_length;
+    const char* source = input.data();
+    size_t source_length = input.length();
 
     if (converter->unicode_ && !converter->ignoreBOM_ && !converter->bomSeen_) {
       int32_t bomOffset = 0;
@@ -455,8 +454,7 @@ void Transcode(const FunctionCallbackInfo<Value>&args) {
   UErrorCode status = U_ZERO_ERROR;
   MaybeLocal<Object> result;
 
-  CHECK(Buffer::HasInstance(args[0]));
-  SPREAD_BUFFER_ARG(args[0], ts_obj);
+  ArrayBufferViewContents<char> input(args[0]);
   const enum encoding fromEncoding = ParseEncoding(isolate, args[1], BUFFER);
   const enum encoding toEncoding = ParseEncoding(isolate, args[2], BUFFER);
 
@@ -490,7 +488,7 @@ void Transcode(const FunctionCallbackInfo<Value>&args) {
     }
 
     result = tfn(env, EncodingName(fromEncoding), EncodingName(toEncoding),
-                 ts_obj_data, ts_obj_length, &status);
+                 input.data(), input.length(), &status);
   } else {
     status = U_ILLEGAL_ARGUMENT_ERROR;
   }
diff --git a/src/node_serdes.cc b/src/node_serdes.cc
index 41ee8afd8c..a2d185c416 100644
--- a/src/node_serdes.cc
+++ b/src/node_serdes.cc
@@ -274,8 +274,8 @@ void SerializerContext::WriteRawBytes(const FunctionCallbackInfo<Value>& args) {
         ctx->env(), "source must be a TypedArray or a DataView");
   }
 
-  ctx->serializer_.WriteRawBytes(Buffer::Data(args[0]),
-                                 Buffer::Length(args[0]));
+  ArrayBufferViewContents<char> bytes(args[0]);
+  ctx->serializer_.WriteRawBytes(bytes.data(), bytes.length());
 }
 
 DeserializerContext::DeserializerContext(Environment* env,
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index cde5419b9c..e9fe969358 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -187,9 +187,9 @@ void TLSWrap::Receive(const FunctionCallbackInfo<Value>& args) {
   TLSWrap* wrap;
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
 
-  CHECK(Buffer::HasInstance(args[0]));
-  char* data = Buffer::Data(args[0]);
-  size_t len = Buffer::Length(args[0]);
+  ArrayBufferViewContents<char> buffer(args[0]);
+  const char* data = buffer.data();
+  size_t len = buffer.length();
   Debug(wrap, "Receiving %zu bytes injected from JS", len);
 
   // Copy given buffer entirely or partiall if handle becomes closed
diff --git a/src/util-inl.h b/src/util-inl.h
index 1abebf6e13..5f8e7acb2a 100644
--- a/src/util-inl.h
+++ b/src/util-inl.h
@@ -497,6 +497,13 @@ ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
 
 template <typename T, size_t S>
 ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
+    v8::Local<v8::Object> value) {
+  CHECK(value->IsArrayBufferView());
+  Read(value.As<v8::ArrayBufferView>());
+}
+
+template <typename T, size_t S>
+ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
     v8::Local<v8::ArrayBufferView> abv) {
   Read(abv);
 }
diff --git a/src/util.h b/src/util.h
index b7a2d28b66..a94e88f232 100644
--- a/src/util.h
+++ b/src/util.h
@@ -446,6 +446,7 @@ class ArrayBufferViewContents {
   ArrayBufferViewContents() = default;
 
   explicit inline ArrayBufferViewContents(v8::Local<v8::Value> value);
+  explicit inline ArrayBufferViewContents(v8::Local<v8::Object> value);
   explicit inline ArrayBufferViewContents(v8::Local<v8::ArrayBufferView> abv);
   inline void Read(v8::Local<v8::ArrayBufferView> abv);