src: provide allocation + nullptr check shortcuts

Provide shortcut `node::CheckedMalloc()` and friends that replace `node::Malloc()` + `CHECK_NE(·, nullptr);` combinations in a few places. PR-URL: nodejs#8482 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Ilkka Myller <[email protected]>
bnoordhuis · Sep 29, 2016 · ea94086 · ea94086
1 parent 48ed654
commit ea94086
Show file tree

Hide file tree

Showing 13 changed files with 67 additions and 49 deletions.
diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc
@@ -174,7 +174,7 @@ static void ares_poll_close_cb(uv_handle_t* watcher) {
 
 /* Allocates and returns a new node_ares_task */
 static node_ares_task* ares_task_create(Environment* env, ares_socket_t sock) {
-  auto task = node::Malloc<node_ares_task>(1);
+  auto task = node::UncheckedMalloc<node_ares_task>(1);
 
   if (task == nullptr) {
     /* Out of memory. */

diff --git a/src/node.cc b/src/node.cc
@@ -979,9 +979,9 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
 
 void* ArrayBufferAllocator::Allocate(size_t size) {
   if (zero_fill_field_ || zero_fill_all_buffers)
-    return node::Calloc(size);
+    return node::UncheckedCalloc(size);
   else
-    return node::Malloc(size);
+    return node::UncheckedMalloc(size);
 }
 
 static bool DomainHasErrorHandler(const Environment* env,

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -89,8 +89,8 @@ bool zero_fill_all_buffers = false;
 namespace {
 
 inline void* BufferMalloc(size_t length) {
-  return zero_fill_all_buffers ? node::Calloc(length) :
-                                 node::Malloc(length);
+  return zero_fill_all_buffers ? node::UncheckedCalloc(length) :
+                                 node::UncheckedMalloc(length);
 }
 
 }  // namespace
@@ -285,7 +285,6 @@ MaybeLocal<Object> New(Isolate* isolate,
       data = nullptr;
     } else if (actual < length) {
       data = node::Realloc(data, actual);
-      CHECK_NE(data, nullptr);
     }
   }
 
@@ -363,7 +362,7 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
   void* new_data;
   if (length > 0) {
     CHECK_NE(data, nullptr);
-    new_data = node::Malloc(length);
+    new_data = node::UncheckedMalloc(length);
     if (new_data == nullptr)
       return Local<Object>();
     memcpy(new_data, data, length);
@@ -1086,7 +1085,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
                           offset,
                           is_forward);
   } else if (enc == LATIN1) {
-    uint8_t* needle_data = node::Malloc<uint8_t>(needle_length);
+    uint8_t* needle_data = node::UncheckedMalloc<uint8_t>(needle_length);
     if (needle_data == nullptr) {
       return args.GetReturnValue().Set(-1);
     }

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -2280,7 +2280,6 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
 
     // OpenSSL takes control of the pointer after accepting it
     char* data = node::Malloc(len);
-    CHECK_NE(data, nullptr);
     memcpy(data, resp, len);
 
     if (!SSL_set_tlsext_status_ocsp_resp(s, data, len))
@@ -3331,7 +3330,6 @@ bool CipherBase::GetAuthTag(char** out, unsigned int* out_len) const {
     return false;
   *out_len = auth_tag_len_;
   *out = node::Malloc(auth_tag_len_);
-  CHECK_NE(*out, nullptr);
   memcpy(*out, auth_tag_, auth_tag_len_);
   return true;
 }
@@ -4907,7 +4905,6 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
   int field_size = EC_GROUP_get_degree(ecdh->group_);
   size_t out_len = (field_size + 7) / 8;
   char* out = node::Malloc(out_len);
-  CHECK_NE(out, nullptr);
 
   int r = ECDH_compute_key(out, out_len, pub, ecdh->key_, nullptr);
   EC_POINT_free(pub);
@@ -4943,7 +4940,6 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
     return env->ThrowError("Failed to get public key length");
 
   unsigned char* out = node::Malloc<unsigned char>(size);
-  CHECK_NE(out, nullptr);
 
   int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr);
   if (r != size) {
@@ -4969,7 +4965,6 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
 
   int size = BN_num_bytes(b);
   unsigned char* out = node::Malloc<unsigned char>(size);
-  CHECK_NE(out, nullptr);
 
   if (size != BN_bn2bin(b, out)) {
     free(out);
@@ -5101,8 +5096,6 @@ class PBKDF2Request : public AsyncWrap {
         keylen_(keylen),
         key_(node::Malloc(keylen)),
         iter_(iter) {
-    if (key() == nullptr)
-      FatalError("node::PBKDF2Request()", "Out of Memory");
     Wrap(object, this);
   }
 
@@ -5263,9 +5256,6 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
   THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Salt");
 
   pass = node::Malloc(passlen);
-  if (pass == nullptr) {
-    FatalError("node::PBKDF2()", "Out of Memory");
-  }
   memcpy(pass, Buffer::Data(args[0]), passlen);
 
   saltlen = Buffer::Length(args[1]);
@@ -5275,9 +5265,6 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
   }
 
   salt = node::Malloc(saltlen);
-  if (salt == nullptr) {
-    FatalError("node::PBKDF2()", "Out of Memory");
-  }
   memcpy(salt, Buffer::Data(args[1]), saltlen);
 
   if (!args[2]->IsNumber()) {
@@ -5368,8 +5355,6 @@ class RandomBytesRequest : public AsyncWrap {
         error_(0),
         size_(size),
         data_(node::Malloc(size)) {
-    if (data() == nullptr)
-      FatalError("node::RandomBytesRequest()", "Out of Memory");
     Wrap(object, this);
   }
 
@@ -5596,8 +5581,6 @@ void GetCurves(const FunctionCallbackInfo<Value>& args) {
   if (num_curves) {
     curves = node::Malloc<EC_builtin_curve>(num_curves);
 
-    CHECK_NE(curves, nullptr);
-
     if (EC_get_builtin_curves(curves, num_curves)) {
       for (size_t i = 0; i < num_curves; i++) {
         arr->Set(i, OneByteString(env->isolate(), OBJ_nid2sn(curves[i].nid)));

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -160,7 +160,7 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
 
   virtual void* Allocate(size_t size);  // Defined in src/node.cc
   virtual void* AllocateUninitialized(size_t size)
-    { return node::Malloc(size); }
+    { return node::UncheckedMalloc(size); }
   virtual void Free(void* data, size_t) { free(data); }
 
  private:

diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
@@ -150,12 +150,6 @@ void StreamWrap::OnAlloc(uv_handle_t* handle,
 void StreamWrap::OnAllocImpl(size_t size, uv_buf_t* buf, void* ctx) {
   buf->base = node::Malloc(size);
   buf->len = size;
-
-  if (buf->base == nullptr && size > 0) {
-    FatalError(
-        "node::StreamWrap::DoAlloc(size_t, uv_buf_t*, void*)",
-        "Out Of Memory");
-  }
 }
 
 
@@ -204,8 +198,8 @@ void StreamWrap::OnReadImpl(ssize_t nread,
     return;
   }
 
-  char* base = node::Realloc(buf->base, nread);
   CHECK_LE(static_cast<size_t>(nread), buf->len);
+  char* base = node::Realloc(buf->base, nread);
 
   if (pending == UV_TCP) {
     pending_obj = AcceptHandle<TCPWrap, uv_tcp_t>(env, wrap);

diff --git a/src/string_bytes.cc b/src/string_bytes.cc
@@ -53,7 +53,7 @@ class ExternString: public ResourceType {
     if (length == 0)
       return scope.Escape(String::Empty(isolate));
 
-    TypeName* new_data = node::Malloc<TypeName>(length);
+    TypeName* new_data = node::UncheckedMalloc<TypeName>(length);
     if (new_data == nullptr) {
       return Local<String>();
     }
@@ -623,7 +623,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
 
     case ASCII:
       if (contains_non_ascii(buf, buflen)) {
-        char* out = node::Malloc(buflen);
+        char* out = node::UncheckedMalloc(buflen);
         if (out == nullptr) {
           return Local<String>();
         }
@@ -658,7 +658,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
 
     case BASE64: {
       size_t dlen = base64_encoded_size(buflen);
-      char* dst = node::Malloc(dlen);
+      char* dst = node::UncheckedMalloc(dlen);
       if (dst == nullptr) {
         return Local<String>();
       }
@@ -677,7 +677,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
 
     case HEX: {
       size_t dlen = buflen * 2;
-      char* dst = node::Malloc(dlen);
+      char* dst = node::UncheckedMalloc(dlen);
       if (dst == nullptr) {
         return Local<String>();
       }

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -662,7 +662,6 @@ void TLSWrap::OnReadImpl(ssize_t nread,
 
 void TLSWrap::OnAllocSelf(size_t suggested_size, uv_buf_t* buf, void* ctx) {
   buf->base = node::Malloc(suggested_size);
-  CHECK_NE(buf->base, nullptr);
   buf->len = suggested_size;
 }
 

diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc
@@ -376,11 +376,6 @@ void UDPWrap::OnAlloc(uv_handle_t* handle,
                       uv_buf_t* buf) {
   buf->base = node::Malloc(suggested_size);
   buf->len = suggested_size;
-
-  if (buf->base == nullptr && suggested_size > 0) {
-    FatalError("node::UDPWrap::OnAlloc(uv_handle_t*, size_t, uv_buf_t*)",
-               "Out Of Memory");
-  }
 }
 
 
@@ -416,7 +411,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle,
     return;
   }
 
-  char* base = node::Realloc(buf->base, nread);
+  char* base = node::UncheckedRealloc(buf->base, nread);
   argv[2] = Buffer::New(env, base, nread).ToLocalChecked();
   argv[3] = AddressToJS(env, addr);
   wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv);

diff --git a/src/util-inl.h b/src/util-inl.h
@@ -245,7 +245,7 @@ inline size_t MultiplyWithOverflowCheck(size_t a, size_t b) {
 // nullptr for zero-sized allocation requests.  Normalize by always using
 // a nullptr.
 template <typename T>
-T* Realloc(T* pointer, size_t n) {
+T* UncheckedRealloc(T* pointer, size_t n) {
   size_t full_size = MultiplyWithOverflowCheck(sizeof(T), n);
 
   if (full_size == 0) {
@@ -258,18 +258,39 @@ T* Realloc(T* pointer, size_t n) {
 
 // As per spec realloc behaves like malloc if passed nullptr.
 template <typename T>
-T* Malloc(size_t n) {
+T* UncheckedMalloc(size_t n) {
   if (n == 0) n = 1;
-  return Realloc<T>(nullptr, n);
+  return UncheckedRealloc<T>(nullptr, n);
 }
 
 template <typename T>
-T* Calloc(size_t n) {
+T* UncheckedCalloc(size_t n) {
   if (n == 0) n = 1;
   MultiplyWithOverflowCheck(sizeof(T), n);
   return static_cast<T*>(calloc(n, sizeof(T)));
 }
 
+template <typename T>
+T* Realloc(T* pointer, size_t n) {
+  T* ret = UncheckedRealloc(pointer, n);
+  if (n > 0) CHECK_NE(ret, nullptr);
+  return ret;
+}
+
+template <typename T>
+T* Malloc(size_t n) {
+  T* ret = UncheckedMalloc<T>(n);
+  if (n > 0) CHECK_NE(ret, nullptr);
+  return ret;
+}
+
+template <typename T>
+T* Calloc(size_t n) {
+  T* ret = UncheckedCalloc<T>(n);
+  if (n > 0) CHECK_NE(ret, nullptr);
+  return ret;
+}
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

diff --git a/src/util.cc b/src/util.cc
@@ -1,6 +1,7 @@
 #include "util.h"
 #include "string_bytes.h"
 #include "node_buffer.h"
+#include "node_internals.h"
 #include <stdio.h>
 
 namespace node {

diff --git a/src/util.h b/src/util.h
@@ -23,6 +23,15 @@ namespace node {
 // nullptr for zero-sized allocation requests.  Normalize by always using
 // a nullptr.
 template <typename T>
+inline T* UncheckedRealloc(T* pointer, size_t n);
+template <typename T>
+inline T* UncheckedMalloc(size_t n);
+template <typename T>
+inline T* UncheckedCalloc(size_t n);
+
+// Same things, but aborts immediately instead of returning nullptr when
+// no memory is available.
+template <typename T>
 inline T* Realloc(T* pointer, size_t n);
 template <typename T>
 inline T* Malloc(size_t n);
@@ -32,6 +41,8 @@ inline T* Calloc(size_t n);
 // Shortcuts for char*.
 inline char* Malloc(size_t n) { return Malloc<char>(n); }
 inline char* Calloc(size_t n) { return Calloc<char>(n); }
+inline char* UncheckedMalloc(size_t n) { return UncheckedMalloc<char>(n); }
+inline char* UncheckedCalloc(size_t n) { return UncheckedCalloc<char>(n); }
 
 #ifdef __GNUC__
 #define NO_RETURN __attribute__((noreturn))
@@ -293,7 +304,6 @@ class MaybeStackBuffer {
       buf_ = buf_st_;
     } else {
       buf_ = Malloc<T>(storage);
-      CHECK_NE(buf_, nullptr);
     }
 
     // Remember how much was allocated to check against that in SetLength().

diff --git a/test/cctest/util.cc b/test/cctest/util.cc
@@ -105,3 +105,19 @@ TEST(UtilTest, Calloc) {
   EXPECT_NE(nullptr, Calloc(0));
   EXPECT_NE(nullptr, Calloc(1));
 }
+
+TEST(UtilTest, UncheckedMalloc) {
+  using node::UncheckedMalloc;
+  EXPECT_NE(nullptr, UncheckedMalloc<char>(0));
+  EXPECT_NE(nullptr, UncheckedMalloc<char>(1));
+  EXPECT_NE(nullptr, UncheckedMalloc(0));
+  EXPECT_NE(nullptr, UncheckedMalloc(1));
+}
+
+TEST(UtilTest, UncheckedCalloc) {
+  using node::UncheckedCalloc;
+  EXPECT_NE(nullptr, UncheckedCalloc<char>(0));
+  EXPECT_NE(nullptr, UncheckedCalloc<char>(1));
+  EXPECT_NE(nullptr, UncheckedCalloc(0));
+  EXPECT_NE(nullptr, UncheckedCalloc(1));
+}