Skip to content

Commit

Permalink
src: provide allocation + nullptr check shortcuts
Browse files Browse the repository at this point in the history
Provide shortcut `node::CheckedMalloc()` and friends that
replace `node::Malloc()` + `CHECK_NE(·, nullptr);` combinations
in a few places.

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 jasnell committed Sep 29, 2016
1 parent d2470d4 commit aed9792
Show file tree
Hide file tree
Showing 13 changed files with 67 additions and 49 deletions.
2 changes: 1 addition & 1 deletion src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
4 changes: 2 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 4 additions & 5 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -285,7 +285,6 @@ MaybeLocal<Object> New(Isolate* isolate,
data = nullptr;
} else if (actual < length) {
data = node::Realloc(data, actual);
CHECK_NE(data, nullptr);
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
17 changes: 0 additions & 17 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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]);
Expand All @@ -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()) {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)));
Expand Down
2 changes: 1 addition & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 1 addition & 7 deletions src/stream_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}


Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
}
Expand Down Expand Up @@ -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>();
}
Expand Down Expand Up @@ -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>();
}
Expand All @@ -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>();
}
Expand Down
1 change: 0 additions & 1 deletion src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
7 changes: 1 addition & 6 deletions src/udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}


Expand Down Expand Up @@ -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);
Expand Down
29 changes: 25 additions & 4 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/util.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "util.h"
#include "string_bytes.h"
#include "node_buffer.h"
#include "node_internals.h"
#include <stdio.h>

namespace node {
Expand Down
12 changes: 11 additions & 1 deletion src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,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);
Expand All @@ -41,6 +50,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))
Expand Down Expand Up @@ -306,7 +317,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().
Expand Down
16 changes: 16 additions & 0 deletions test/cctest/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

0 comments on commit aed9792

Please sign in to comment.