Skip to content

Commit

Permalink
src: remove UncheckedMalloc(0) workaround
Browse files Browse the repository at this point in the history
Assuming that UncheckedMalloc(0) returns a non-nullptr is non-standard
and we use other allocators as well (e.g., OPENSSL_malloc) that do not
guarantee this behavior. It is the caller's responsibility to check that
size != 0 implies UncheckedMalloc(size) != nullptr, and that's exactly
what the checked variants (Malloc etc.) already do.

The current behavior is also inconsistent with UncheckedRealloc(), which
always returns a nullptr when the size is 0, and with the documentation
in src/README.md.

Refs: nodejs#8571
Refs: nodejs#8572
  • Loading branch information
tniessen committed Sep 6, 2022
1 parent addb726 commit 7d5bd1d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 23 deletions.
2 changes: 0 additions & 2 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,11 @@ T* UncheckedRealloc(T* pointer, size_t n) {
// As per spec realloc behaves like malloc if passed nullptr.
template <typename T>
inline T* UncheckedMalloc(size_t n) {
if (n == 0) n = 1;
return UncheckedRealloc<T>(nullptr, n);
}

template <typename T>
inline T* UncheckedCalloc(size_t n) {
if (n == 0) n = 1;
MultiplyWithOverflowCheck(sizeof(T), n);
return static_cast<T*>(calloc(n, sizeof(T)));
}
Expand Down
42 changes: 21 additions & 21 deletions test/cctest/test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,39 +97,39 @@ TEST(UtilTest, ToLower) {
EXPECT_EQ('a', ToLower('A'));
}

#define TEST_AND_FREE(expression) \
do { \
auto pointer = expression; \
EXPECT_NE(nullptr, pointer); \
free(pointer); \
#define TEST_AND_FREE(expression, size) \
do { \
auto pointer = expression(size); \
if (size != 0) EXPECT_NE(pointer, nullptr); \
free(pointer); \
} while (0)

TEST(UtilTest, Malloc) {
TEST_AND_FREE(Malloc<char>(0));
TEST_AND_FREE(Malloc<char>(1));
TEST_AND_FREE(Malloc(0));
TEST_AND_FREE(Malloc(1));
TEST_AND_FREE(Malloc<char>, 0);
TEST_AND_FREE(Malloc<char>, 1);
TEST_AND_FREE(Malloc, 0);
TEST_AND_FREE(Malloc, 1);
}

TEST(UtilTest, Calloc) {
TEST_AND_FREE(Calloc<char>(0));
TEST_AND_FREE(Calloc<char>(1));
TEST_AND_FREE(Calloc(0));
TEST_AND_FREE(Calloc(1));
TEST_AND_FREE(Calloc<char>, 0);
TEST_AND_FREE(Calloc<char>, 1);
TEST_AND_FREE(Calloc, 0);
TEST_AND_FREE(Calloc, 1);
}

TEST(UtilTest, UncheckedMalloc) {
TEST_AND_FREE(UncheckedMalloc<char>(0));
TEST_AND_FREE(UncheckedMalloc<char>(1));
TEST_AND_FREE(UncheckedMalloc(0));
TEST_AND_FREE(UncheckedMalloc(1));
TEST_AND_FREE(UncheckedMalloc<char>, 0);
TEST_AND_FREE(UncheckedMalloc<char>, 1);
TEST_AND_FREE(UncheckedMalloc, 0);
TEST_AND_FREE(UncheckedMalloc, 1);
}

TEST(UtilTest, UncheckedCalloc) {
TEST_AND_FREE(UncheckedCalloc<char>(0));
TEST_AND_FREE(UncheckedCalloc<char>(1));
TEST_AND_FREE(UncheckedCalloc(0));
TEST_AND_FREE(UncheckedCalloc(1));
TEST_AND_FREE(UncheckedCalloc<char>, 0);
TEST_AND_FREE(UncheckedCalloc<char>, 1);
TEST_AND_FREE(UncheckedCalloc, 0);
TEST_AND_FREE(UncheckedCalloc, 1);
}

template <typename T>
Expand Down

0 comments on commit 7d5bd1d

Please sign in to comment.