From d2470d4dff55726e88fe59b3e21aee2a006e2a55 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 10 Sep 2016 17:50:02 +0200 Subject: [PATCH] src: pass desired return type to allocators Pass the desired return type directly to the allocation functions, so that the resulting `static_cast` from `void*` becomes unneccessary and the return type can be use as a reasonable default value for the `size` parameter. PR-URL: https://github.com/nodejs/node/pull/8482 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Michael Dawson Reviewed-By: Ilkka Myller --- src/cares_wrap.cc | 3 +-- src/node.cc | 2 +- src/node_buffer.cc | 20 +++++++++++++------- src/node_crypto.cc | 21 ++++++++++----------- src/stream_wrap.cc | 4 ++-- src/string_bytes.cc | 9 ++++----- src/tls_wrap.cc | 2 +- src/udp_wrap.cc | 4 ++-- src/util-inl.h | 21 +++++++++++---------- src/util.h | 15 +++++++++++---- test/cctest/util.cc | 12 +++++++----- 11 files changed, 63 insertions(+), 50 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 3aab61e1b360fc..faa5f74e21e8a2 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -174,8 +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) { - node_ares_task* task = - static_cast(node::Malloc(sizeof(*task))); + auto task = node::Malloc(1); if (task == nullptr) { /* Out of memory. */ diff --git a/src/node.cc b/src/node.cc index be0c42b19f7b39..468fe9a165c167 100644 --- a/src/node.cc +++ b/src/node.cc @@ -979,7 +979,7 @@ Local WinapiErrnoException(Isolate* isolate, void* ArrayBufferAllocator::Allocate(size_t size) { if (zero_fill_field_ || zero_fill_all_buffers) - return node::Calloc(size, 1); + return node::Calloc(size); else return node::Malloc(size); } diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 55004a36b7bfc8..1ab664d364526d 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -49,9 +49,6 @@ THROW_AND_RETURN_IF_OOB(end <= end_max); \ size_t length = end - start; -#define BUFFER_MALLOC(length) \ - zero_fill_all_buffers ? node::Calloc(length, 1) : node::Malloc(length) - #if defined(__GNUC__) || defined(__clang__) #define BSWAP_INTRINSIC_2(x) __builtin_bswap16(x) #define BSWAP_INTRINSIC_4(x) __builtin_bswap32(x) @@ -89,6 +86,15 @@ namespace node { // if true, all Buffer and SlowBuffer instances will automatically zero-fill bool zero_fill_all_buffers = false; +namespace { + +inline void* BufferMalloc(size_t length) { + return zero_fill_all_buffers ? node::Calloc(length) : + node::Malloc(length); +} + +} // namespace + namespace Buffer { using v8::ArrayBuffer; @@ -266,7 +272,7 @@ MaybeLocal New(Isolate* isolate, char* data = nullptr; if (length > 0) { - data = static_cast(BUFFER_MALLOC(length)); + data = static_cast(BufferMalloc(length)); if (data == nullptr) return Local(); @@ -278,7 +284,7 @@ MaybeLocal New(Isolate* isolate, free(data); data = nullptr; } else if (actual < length) { - data = static_cast(node::Realloc(data, actual)); + data = node::Realloc(data, actual); CHECK_NE(data, nullptr); } } @@ -312,7 +318,7 @@ MaybeLocal New(Environment* env, size_t length) { void* data; if (length > 0) { - data = BUFFER_MALLOC(length); + data = BufferMalloc(length); if (data == nullptr) return Local(); } else { @@ -1080,7 +1086,7 @@ void IndexOfString(const FunctionCallbackInfo& args) { offset, is_forward); } else if (enc == LATIN1) { - uint8_t* needle_data = static_cast(node::Malloc(needle_length)); + uint8_t* needle_data = node::Malloc(needle_length); if (needle_data == nullptr) { return args.GetReturnValue().Set(-1); } diff --git a/src/node_crypto.cc b/src/node_crypto.cc index d051305dd84d8d..cbcbcc70cfd34c 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2279,7 +2279,7 @@ int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { size_t len = Buffer::Length(obj); // OpenSSL takes control of the pointer after accepting it - char* data = reinterpret_cast(node::Malloc(len)); + char* data = node::Malloc(len); CHECK_NE(data, nullptr); memcpy(data, resp, len); @@ -3330,7 +3330,7 @@ bool CipherBase::GetAuthTag(char** out, unsigned int* out_len) const { if (initialised_ || kind_ != kCipher || !auth_tag_) return false; *out_len = auth_tag_len_; - *out = static_cast(node::Malloc(auth_tag_len_)); + *out = node::Malloc(auth_tag_len_); CHECK_NE(*out, nullptr); memcpy(*out, auth_tag_, auth_tag_len_); return true; @@ -4906,7 +4906,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { // NOTE: field_size is in bits int field_size = EC_GROUP_get_degree(ecdh->group_); size_t out_len = (field_size + 7) / 8; - char* out = static_cast(node::Malloc(out_len)); + char* out = node::Malloc(out_len); CHECK_NE(out, nullptr); int r = ECDH_compute_key(out, out_len, pub, ecdh->key_, nullptr); @@ -4942,7 +4942,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo& args) { if (size == 0) return env->ThrowError("Failed to get public key length"); - unsigned char* out = static_cast(node::Malloc(size)); + unsigned char* out = node::Malloc(size); CHECK_NE(out, nullptr); int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr); @@ -4968,7 +4968,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { return env->ThrowError("Failed to get ECDH private key"); int size = BN_num_bytes(b); - unsigned char* out = static_cast(node::Malloc(size)); + unsigned char* out = node::Malloc(size); CHECK_NE(out, nullptr); if (size != BN_bn2bin(b, out)) { @@ -5099,7 +5099,7 @@ class PBKDF2Request : public AsyncWrap { saltlen_(saltlen), salt_(salt), keylen_(keylen), - key_(static_cast(node::Malloc(keylen))), + key_(node::Malloc(keylen)), iter_(iter) { if (key() == nullptr) FatalError("node::PBKDF2Request()", "Out of Memory"); @@ -5262,7 +5262,7 @@ void PBKDF2(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Salt"); - pass = static_cast(node::Malloc(passlen)); + pass = node::Malloc(passlen); if (pass == nullptr) { FatalError("node::PBKDF2()", "Out of Memory"); } @@ -5274,7 +5274,7 @@ void PBKDF2(const FunctionCallbackInfo& args) { goto err; } - salt = static_cast(node::Malloc(saltlen)); + salt = node::Malloc(saltlen); if (salt == nullptr) { FatalError("node::PBKDF2()", "Out of Memory"); } @@ -5367,7 +5367,7 @@ class RandomBytesRequest : public AsyncWrap { : AsyncWrap(env, object, AsyncWrap::PROVIDER_CRYPTO), error_(0), size_(size), - data_(static_cast(node::Malloc(size))) { + data_(node::Malloc(size)) { if (data() == nullptr) FatalError("node::RandomBytesRequest()", "Out of Memory"); Wrap(object, this); @@ -5594,8 +5594,7 @@ void GetCurves(const FunctionCallbackInfo& args) { EC_builtin_curve* curves; if (num_curves) { - curves = static_cast(node::Malloc(sizeof(*curves), - num_curves)); + curves = node::Malloc(num_curves); CHECK_NE(curves, nullptr); diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index d294b641ffa444..c3233e4064b309 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -148,7 +148,7 @@ void StreamWrap::OnAlloc(uv_handle_t* handle, void StreamWrap::OnAllocImpl(size_t size, uv_buf_t* buf, void* ctx) { - buf->base = static_cast(node::Malloc(size)); + buf->base = node::Malloc(size); buf->len = size; if (buf->base == nullptr && size > 0) { @@ -204,7 +204,7 @@ void StreamWrap::OnReadImpl(ssize_t nread, return; } - char* base = static_cast(node::Realloc(buf->base, nread)); + char* base = node::Realloc(buf->base, nread); CHECK_LE(static_cast(nread), buf->len); if (pending == UV_TCP) { diff --git a/src/string_bytes.cc b/src/string_bytes.cc index fddf61de636306..8acba5d55d1ee1 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -53,8 +53,7 @@ class ExternString: public ResourceType { if (length == 0) return scope.Escape(String::Empty(isolate)); - TypeName* new_data = - static_cast(node::Malloc(length, sizeof(*new_data))); + TypeName* new_data = node::Malloc(length); if (new_data == nullptr) { return Local(); } @@ -624,7 +623,7 @@ Local StringBytes::Encode(Isolate* isolate, case ASCII: if (contains_non_ascii(buf, buflen)) { - char* out = static_cast(node::Malloc(buflen)); + char* out = node::Malloc(buflen); if (out == nullptr) { return Local(); } @@ -659,7 +658,7 @@ Local StringBytes::Encode(Isolate* isolate, case BASE64: { size_t dlen = base64_encoded_size(buflen); - char* dst = static_cast(node::Malloc(dlen)); + char* dst = node::Malloc(dlen); if (dst == nullptr) { return Local(); } @@ -678,7 +677,7 @@ Local StringBytes::Encode(Isolate* isolate, case HEX: { size_t dlen = buflen * 2; - char* dst = static_cast(node::Malloc(dlen)); + char* dst = node::Malloc(dlen); if (dst == nullptr) { return Local(); } diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index d1b1aeccdd95b0..dbe6677516afb0 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -661,7 +661,7 @@ void TLSWrap::OnReadImpl(ssize_t nread, void TLSWrap::OnAllocSelf(size_t suggested_size, uv_buf_t* buf, void* ctx) { - buf->base = static_cast(node::Malloc(suggested_size)); + 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 index c8009a5276e228..bbd30a5266e443 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -374,7 +374,7 @@ void UDPWrap::OnSend(uv_udp_send_t* req, int status) { void UDPWrap::OnAlloc(uv_handle_t* handle, size_t suggested_size, uv_buf_t* buf) { - buf->base = static_cast(node::Malloc(suggested_size)); + buf->base = node::Malloc(suggested_size); buf->len = suggested_size; if (buf->base == nullptr && suggested_size > 0) { @@ -416,7 +416,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle, return; } - char* base = static_cast(node::Realloc(buf->base, nread)); + char* base = node::Realloc(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 index 3f3139e1f05fbf..3a5c7622c73cdf 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -244,29 +244,30 @@ inline size_t MultiplyWithOverflowCheck(size_t a, size_t b) { // that the standard allows them to either return a unique pointer or a // nullptr for zero-sized allocation requests. Normalize by always using // a nullptr. -void* Realloc(void* pointer, size_t n, size_t size) { - size_t full_size = MultiplyWithOverflowCheck(size, n); +template +T* Realloc(T* pointer, size_t n) { + size_t full_size = MultiplyWithOverflowCheck(sizeof(T), n); if (full_size == 0) { free(pointer); return nullptr; } - return realloc(pointer, full_size); + return static_cast(realloc(pointer, full_size)); } // As per spec realloc behaves like malloc if passed nullptr. -void* Malloc(size_t n, size_t size) { +template +T* Malloc(size_t n) { if (n == 0) n = 1; - if (size == 0) size = 1; - return Realloc(nullptr, n, size); + return Realloc(nullptr, n); } -void* Calloc(size_t n, size_t size) { +template +T* Calloc(size_t n) { if (n == 0) n = 1; - if (size == 0) size = 1; - MultiplyWithOverflowCheck(size, n); - return calloc(n, size); + MultiplyWithOverflowCheck(sizeof(T), n); + return static_cast(calloc(n, sizeof(T))); } } // namespace node diff --git a/src/util.h b/src/util.h index f240baf3fae7a0..2ea80c36ae57e4 100644 --- a/src/util.h +++ b/src/util.h @@ -31,9 +31,16 @@ namespace node { // that the standard allows them to either return a unique pointer or a // nullptr for zero-sized allocation requests. Normalize by always using // a nullptr. -inline void* Realloc(void* pointer, size_t n, size_t size = 1); -inline void* Malloc(size_t n, size_t size = 1); -inline void* Calloc(size_t n, size_t size = 1); +template +inline T* Realloc(T* pointer, size_t n); +template +inline T* Malloc(size_t n); +template +inline T* Calloc(size_t n); + +// Shortcuts for char*. +inline char* Malloc(size_t n) { return Malloc(n); } +inline char* Calloc(size_t n) { return Calloc(n); } #ifdef __GNUC__ #define NO_RETURN __attribute__((noreturn)) @@ -298,7 +305,7 @@ class MaybeStackBuffer { if (storage <= kStackStorageSize) { buf_ = buf_st_; } else { - buf_ = static_cast(Malloc(sizeof(T), storage)); + buf_ = Malloc(storage); CHECK_NE(buf_, nullptr); } diff --git a/test/cctest/util.cc b/test/cctest/util.cc index 79f1524660b213..7bbf53af13d3c4 100644 --- a/test/cctest/util.cc +++ b/test/cctest/util.cc @@ -92,14 +92,16 @@ TEST(UtilTest, ToLower) { TEST(UtilTest, Malloc) { using node::Malloc; + EXPECT_NE(nullptr, Malloc(0)); + EXPECT_NE(nullptr, Malloc(1)); EXPECT_NE(nullptr, Malloc(0)); EXPECT_NE(nullptr, Malloc(1)); } TEST(UtilTest, Calloc) { using node::Calloc; - EXPECT_NE(nullptr, Calloc(0, 0)); - EXPECT_NE(nullptr, Calloc(1, 0)); - EXPECT_NE(nullptr, Calloc(0, 1)); - EXPECT_NE(nullptr, Calloc(1, 1)); -} \ No newline at end of file + EXPECT_NE(nullptr, Calloc(0)); + EXPECT_NE(nullptr, Calloc(1)); + EXPECT_NE(nullptr, Calloc(0)); + EXPECT_NE(nullptr, Calloc(1)); +}