Skip to content

Commit

Permalink
src: pass desired return type to allocators
Browse files Browse the repository at this point in the history
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.

Backport-PR-URL: #16587
PR-URL: #8482
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
  • Loading branch information
addaleax authored and MylesBorins committed Nov 14, 2017
1 parent 19f3ac9 commit 4aec8cf
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 57 deletions.
17 changes: 7 additions & 10 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,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_ares_task*>(node::Malloc(sizeof(*task)));
auto task = node::Malloc<node_ares_task>(1);

if (task == nullptr) {
/* Out of memory. */
Expand Down Expand Up @@ -329,11 +328,10 @@ void cares_wrap_hostent_cpy(struct hostent* dest, struct hostent* src) {
alias_count++) {
}

dest->h_aliases = static_cast<char**>(node::Malloc((alias_count + 1) *
sizeof(char*)));
dest->h_aliases = node::Malloc<char*>(alias_count + 1);
for (size_t i = 0; i < alias_count; i++) {
cur_alias_length = strlen(src->h_aliases[i]);
dest->h_aliases[i] = static_cast<char*>(node::Malloc(cur_alias_length + 1));
dest->h_aliases[i] = node::Malloc(cur_alias_length + 1);
memcpy(dest->h_aliases[i], src->h_aliases[i], cur_alias_length + 1);
}
dest->h_aliases[alias_count] = nullptr;
Expand All @@ -345,10 +343,9 @@ void cares_wrap_hostent_cpy(struct hostent* dest, struct hostent* src) {
list_count++) {
}

dest->h_addr_list = static_cast<char**>(node::Malloc((list_count + 1) *
sizeof(char*)));
dest->h_addr_list = node::Malloc<char*>(list_count + 1);
for (size_t i = 0; i < list_count; i++) {
dest->h_addr_list[i] = static_cast<char*>(node::Malloc(src->h_length));
dest->h_addr_list[i] = node::Malloc(src->h_length);
memcpy(dest->h_addr_list[i], src->h_addr_list[i], src->h_length);
}
dest->h_addr_list[list_count] = nullptr;
Expand Down Expand Up @@ -507,7 +504,7 @@ class QueryWrap : public AsyncWrap {

unsigned char* buf_copy = nullptr;
if (status == ARES_SUCCESS) {
buf_copy = static_cast<unsigned char*>(node::Malloc(answer_len));
buf_copy = node::Malloc<unsigned char>(answer_len);
memcpy(buf_copy, answer_buf, answer_len);
}

Expand All @@ -534,7 +531,7 @@ class QueryWrap : public AsyncWrap {

struct hostent* host_copy = nullptr;
if (status == ARES_SUCCESS) {
host_copy = static_cast<hostent*>(node::Malloc(sizeof(hostent)));
host_copy = node::Malloc<hostent>(1);
cares_wrap_hostent_cpy(host_copy, host);
}

Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ void* ArrayBufferAllocator::Allocate(size_t size) {
if (env_ == nullptr ||
!env_->array_buffer_allocator_info()->no_zero_fill() ||
zero_fill_all_buffers)
return node::Calloc(size, 1);
return node::Calloc(size);
env_->array_buffer_allocator_info()->reset_fill_flag();
return node::Malloc(size);
}
Expand Down
20 changes: 13 additions & 7 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,20 @@
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)

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;
Expand Down Expand Up @@ -234,7 +240,7 @@ MaybeLocal<Object> New(Isolate* isolate,
char* data = nullptr;

if (length > 0) {
data = static_cast<char*>(BUFFER_MALLOC(length));
data = static_cast<char*>(BufferMalloc(length));

if (data == nullptr)
return Local<Object>();
Expand All @@ -246,7 +252,7 @@ MaybeLocal<Object> New(Isolate* isolate,
free(data);
data = nullptr;
} else if (actual < length) {
data = static_cast<char*>(node::Realloc(data, actual));
data = node::Realloc(data, actual);
CHECK_NE(data, nullptr);
}
}
Expand Down Expand Up @@ -280,7 +286,7 @@ MaybeLocal<Object> New(Environment* env, size_t length) {

void* data;
if (length > 0) {
data = BUFFER_MALLOC(length);
data = BufferMalloc(length);
if (data == nullptr)
return Local<Object>();
} else {
Expand Down Expand Up @@ -1063,7 +1069,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
offset,
is_forward);
} else if (enc == LATIN1) {
uint8_t* needle_data = static_cast<uint8_t*>(node::Malloc(needle_length));
uint8_t* needle_data = node::Malloc<uint8_t>(needle_length);
if (needle_data == nullptr) {
return args.GetReturnValue().Set(-1);
}
Expand Down
21 changes: 10 additions & 11 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2386,7 +2386,7 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
size_t len = Buffer::Length(obj);

// OpenSSL takes control of the pointer after accepting it
char* data = reinterpret_cast<char*>(node::Malloc(len));
char* data = node::Malloc(len);
CHECK_NE(data, nullptr);
memcpy(data, resp, len);

Expand Down Expand Up @@ -3466,7 +3466,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<char*>(node::Malloc(auth_tag_len_));
*out = node::Malloc(auth_tag_len_);
CHECK_NE(*out, nullptr);
memcpy(*out, auth_tag_, auth_tag_len_);
return true;
Expand Down Expand Up @@ -5138,7 +5138,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& 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<char*>(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);
Expand Down Expand Up @@ -5174,7 +5174,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
if (size == 0)
return env->ThrowError("Failed to get public key length");

unsigned char* out = static_cast<unsigned char*>(node::Malloc(size));
unsigned char* out = node::Malloc<unsigned char>(size);
CHECK_NE(out, nullptr);

int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr);
Expand All @@ -5200,7 +5200,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
return env->ThrowError("Failed to get ECDH private key");

int size = BN_num_bytes(b);
unsigned char* out = static_cast<unsigned char*>(node::Malloc(size));
unsigned char* out = node::Malloc<unsigned char>(size);
CHECK_NE(out, nullptr);

if (size != BN_bn2bin(b, out)) {
Expand Down Expand Up @@ -5333,7 +5333,7 @@ class PBKDF2Request : public AsyncWrap {
saltlen_(saltlen),
salt_(salt),
keylen_(keylen),
key_(static_cast<char*>(node::Malloc(keylen))),
key_(node::Malloc(keylen)),
iter_(iter) {
if (key() == nullptr)
FatalError("node::PBKDF2Request()", "Out of Memory");
Expand Down Expand Up @@ -5496,7 +5496,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {

THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Salt");

pass = static_cast<char*>(node::Malloc(passlen));
pass = node::Malloc(passlen);
if (pass == nullptr) {
FatalError("node::PBKDF2()", "Out of Memory");
}
Expand All @@ -5508,7 +5508,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
goto err;
}

salt = static_cast<char*>(node::Malloc(saltlen));
salt = node::Malloc(saltlen);
if (salt == nullptr) {
FatalError("node::PBKDF2()", "Out of Memory");
}
Expand Down Expand Up @@ -5601,7 +5601,7 @@ class RandomBytesRequest : public AsyncWrap {
: AsyncWrap(env, object, AsyncWrap::PROVIDER_CRYPTO),
error_(0),
size_(size),
data_(static_cast<char*>(node::Malloc(size))) {
data_(node::Malloc(size)) {
if (data() == nullptr)
FatalError("node::RandomBytesRequest()", "Out of Memory");
Wrap(object, this);
Expand Down Expand Up @@ -5828,8 +5828,7 @@ void GetCurves(const FunctionCallbackInfo<Value>& args) {
EC_builtin_curve* curves;

if (num_curves) {
curves = static_cast<EC_builtin_curve*>(node::Malloc(sizeof(*curves),
num_curves));
curves = node::Malloc<EC_builtin_curve>(num_curves);

CHECK_NE(curves, nullptr);

Expand Down
4 changes: 2 additions & 2 deletions src/stream_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<char*>(node::Malloc(size));
buf->base = node::Malloc(size);
buf->len = size;

if (buf->base == nullptr && size > 0) {
Expand Down Expand Up @@ -204,7 +204,7 @@ void StreamWrap::OnReadImpl(ssize_t nread,
return;
}

char* base = static_cast<char*>(node::Realloc(buf->base, nread));
char* base = node::Realloc(buf->base, nread);
CHECK_LE(static_cast<size_t>(nread), buf->len);

if (pending == UV_TCP) {
Expand Down
9 changes: 4 additions & 5 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ class ExternString: public ResourceType {
if (length == 0)
return scope.Escape(String::Empty(isolate));

TypeName* new_data =
static_cast<TypeName*>(node::Malloc(length, sizeof(*new_data)));
TypeName* new_data = node::Malloc<TypeName>(length);
if (new_data == nullptr) {
return Local<String>();
}
Expand Down Expand Up @@ -610,7 +609,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,

case ASCII:
if (contains_non_ascii(buf, buflen)) {
char* out = static_cast<char*>(node::Malloc(buflen));
char* out = node::Malloc(buflen);
if (out == nullptr) {
return Local<String>();
}
Expand Down Expand Up @@ -645,7 +644,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,

case BASE64: {
size_t dlen = base64_encoded_size(buflen);
char* dst = static_cast<char*>(node::Malloc(dlen));
char* dst = node::Malloc(dlen);
if (dst == nullptr) {
return Local<String>();
}
Expand All @@ -664,7 +663,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,

case HEX: {
size_t dlen = buflen * 2;
char* dst = static_cast<char*>(node::Malloc(dlen));
char* dst = node::Malloc(dlen);
if (dst == nullptr) {
return Local<String>();
}
Expand Down
2 changes: 1 addition & 1 deletion src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ void TLSWrap::OnDestructImpl(void* ctx) {


void TLSWrap::OnAllocSelf(size_t suggested_size, uv_buf_t* buf, void* ctx) {
buf->base = static_cast<char*>(node::Malloc(suggested_size));
buf->base = node::Malloc(suggested_size);
CHECK_NE(buf->base, nullptr);
buf->len = suggested_size;
}
Expand Down
4 changes: 2 additions & 2 deletions src/udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,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<char*>(node::Malloc(suggested_size));
buf->base = node::Malloc(suggested_size);
buf->len = suggested_size;

if (buf->base == nullptr && suggested_size > 0) {
Expand Down Expand Up @@ -415,7 +415,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle,
return;
}

char* base = static_cast<char*>(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);
Expand Down
21 changes: 11 additions & 10 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,29 +335,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 <typename T>
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<T*>(realloc(pointer, full_size));
}

// As per spec realloc behaves like malloc if passed nullptr.
void* Malloc(size_t n, size_t size) {
template <typename T>
T* Malloc(size_t n) {
if (n == 0) n = 1;
if (size == 0) size = 1;
return Realloc(nullptr, n, size);
return Realloc<T>(nullptr, n);
}

void* Calloc(size_t n, size_t size) {
template <typename T>
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<T*>(calloc(n, sizeof(T)));
}

} // namespace node
Expand Down
15 changes: 11 additions & 4 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T>
inline T* Realloc(T* pointer, size_t n);
template <typename T>
inline T* Malloc(size_t n);
template <typename T>
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); }

#ifdef __GNUC__
#define NO_RETURN __attribute__((noreturn))
Expand Down Expand Up @@ -294,7 +301,7 @@ class MaybeStackBuffer {
if (storage <= kStackStorageSize) {
buf_ = buf_st_;
} else {
buf_ = static_cast<T*>(Malloc(sizeof(T), storage));
buf_ = Malloc<T>(storage);
CHECK_NE(buf_, nullptr);
}

Expand Down
10 changes: 6 additions & 4 deletions test/cctest/test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,16 @@ TEST(UtilTest, ToLower) {

TEST(UtilTest, Malloc) {
using node::Malloc;
EXPECT_NE(nullptr, Malloc<char>(0));
EXPECT_NE(nullptr, Malloc<char>(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));
EXPECT_NE(nullptr, Calloc<char>(0));
EXPECT_NE(nullptr, Calloc<char>(1));
EXPECT_NE(nullptr, Calloc(0));
EXPECT_NE(nullptr, Calloc(1));
}

0 comments on commit 4aec8cf

Please sign in to comment.