FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: remove UncheckedMalloc(0) workaround · nodejs/node@3abb607 · GitHub

/ node Public

Commit 3abb607

Browse files
tniessen
authored and
danielleadams
committed
src: remove UncheckedMalloc(0) workaround
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 as well as with multiple comments in the source code. This changes UncheckedMalloc(), UncheckedCalloc(), and UncheckedRealloc() to always return a nullptr when the size is 0 instead of doing fake allocations in UncheckedMalloc() and UncheckedCalloc() while returning a nullptr from UncheckedRealloc(). This is consistent with existing documentation and comments. Refs: #8571 Refs: #8572 PR-URL: #44543 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 0606f92 commit 3abb607

3 files changed

Lines changed: 30 additions & 30 deletions

File tree

‎src/string_bytes.cc‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -704,20 +704,21 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
704704
}
705705

706706
case UCS2: {
707+
size_t str_len = buflen / 2;
707708
if (IsBigEndian()) {
708-
uint16_t* dst = node::UncheckedMalloc<uint16_t>(buflen / 2);
709-
if (dst == nullptr) {
709+
uint16_t* dst = node::UncheckedMalloc<uint16_t>(str_len);
710+
if (str_len != 0 && dst == nullptr) {
710711
*error = node::ERR_MEMORY_ALLOCATION_FAILED(isolate);
711712
return MaybeLocal<Value>();
712713
}
713-
for (size_t i = 0, k = 0; k < buflen / 2; i += 2, k += 1) {
714+
for (size_t i = 0, k = 0; k < str_len; i += 2, k += 1) {
714715
// The input is in *little endian*, because that's what Node.js
715716
// expects, so the high byte comes after the low byte.
716717
const uint8_t hi = static_cast<uint8_t>(buf[i + 1]);
717718
const uint8_t lo = static_cast<uint8_t>(buf[i + 0]);
718719
dst[k] = static_cast<uint16_t>(hi) << 8 | lo;
719720
}
720-
return ExternTwoByteString::New(isolate, dst, buflen / 2, error);
721+
return ExternTwoByteString::New(isolate, dst, str_len, error);
721722
}
722723
if (reinterpret_cast<uintptr_t>(buf) % 2 != 0) {
723724
// Unaligned data still means we can't directly pass it to V8.
@@ -728,10 +729,10 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
728729
}
729730
memcpy(dst, buf, buflen);
730731
return ExternTwoByteString::New(
731-
isolate, reinterpret_cast<uint16_t*>(dst), buflen / 2, error);
732+
isolate, reinterpret_cast<uint16_t*>(dst), str_len, error);
732733
}
733734
return ExternTwoByteString::NewFromCopy(
734-
isolate, reinterpret_cast<const uint16_t*>(buf), buflen / 2, error);
735+
isolate, reinterpret_cast<const uint16_t*>(buf), str_len, error);
735736
}
736737

737738
default:
@@ -747,6 +748,7 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
747748
const uint16_t* buf,
748749
size_t buflen,
749750
Local<Value>* error) {
751+
if (buflen == 0) return String::Empty(isolate);
750752
CHECK_BUFLEN_IN_RANGE(buflen);
751753

752754
// Node's "ucs2" encoding expects LE character data inside a

‎src/util-inl.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,14 +361,12 @@ T* UncheckedRealloc(T* pointer, size_t n) {
361361
// As per spec realloc behaves like malloc if passed nullptr.
362362
template <typename T>
363363
inline T* UncheckedMalloc(size_t n) {
364-
if (n == 0) n = 1;
365364
return UncheckedRealloc<T>(nullptr, n);
366365
}
367366

368367
template <typename T>
369368
inline T* UncheckedCalloc(size_t n) {
370-
if (n == 0) n = 1;
371-
MultiplyWithOverflowCheck(sizeof(T), n);
369+
if (MultiplyWithOverflowCheck(sizeof(T), n) == 0) return nullptr;
372370
return static_cast<T*>(calloc(n, sizeof(T)));
373371
}
374372

‎test/cctest/test_util.cc‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -97,39 +97,39 @@ TEST(UtilTest, ToLower) {
9797
EXPECT_EQ('a', ToLower('A'));
9898
}
9999

100-
#define TEST_AND_FREE(expression) \
101-
do { \
102-
auto pointer = expression; \
103-
EXPECT_NE(nullptr, pointer); \
104-
free(pointer); \
100+
#define TEST_AND_FREE(expression, size) \
101+
do { \
102+
auto pointer = expression(size); \
103+
EXPECT_EQ(pointer == nullptr, size == 0); \
104+
free(pointer); \
105105
} while (0)
106106

107107
TEST(UtilTest, Malloc) {
108-
TEST_AND_FREE(Malloc<char>(0));
109-
TEST_AND_FREE(Malloc<char>(1));
110-
TEST_AND_FREE(Malloc(0));
111-
TEST_AND_FREE(Malloc(1));
108+
TEST_AND_FREE(Malloc<char>, 0);
109+
TEST_AND_FREE(Malloc<char>, 1);
110+
TEST_AND_FREE(Malloc, 0);
111+
TEST_AND_FREE(Malloc, 1);
112112
}
113113

114114
TEST(UtilTest, Calloc) {
115-
TEST_AND_FREE(Calloc<char>(0));
116-
TEST_AND_FREE(Calloc<char>(1));
117-
TEST_AND_FREE(Calloc(0));
118-
TEST_AND_FREE(Calloc(1));
115+
TEST_AND_FREE(Calloc<char>, 0);
116+
TEST_AND_FREE(Calloc<char>, 1);
117+
TEST_AND_FREE(Calloc, 0);
118+
TEST_AND_FREE(Calloc, 1);
119119
}
120120

121121
TEST(UtilTest, UncheckedMalloc) {
122-
TEST_AND_FREE(UncheckedMalloc<char>(0));
123-
TEST_AND_FREE(UncheckedMalloc<char>(1));
124-
TEST_AND_FREE(UncheckedMalloc(0));
125-
TEST_AND_FREE(UncheckedMalloc(1));
122+
TEST_AND_FREE(UncheckedMalloc<char>, 0);
123+
TEST_AND_FREE(UncheckedMalloc<char>, 1);
124+
TEST_AND_FREE(UncheckedMalloc, 0);
125+
TEST_AND_FREE(UncheckedMalloc, 1);
126126
}
127127

128128
TEST(UtilTest, UncheckedCalloc) {
129-
TEST_AND_FREE(UncheckedCalloc<char>(0));
130-
TEST_AND_FREE(UncheckedCalloc<char>(1));
131-
TEST_AND_FREE(UncheckedCalloc(0));
132-
TEST_AND_FREE(UncheckedCalloc(1));
129+
TEST_AND_FREE(UncheckedCalloc<char>, 0);
130+
TEST_AND_FREE(UncheckedCalloc<char>, 1);
131+
TEST_AND_FREE(UncheckedCalloc, 0);
132+
TEST_AND_FREE(UncheckedCalloc, 1);
133133
}
134134

135135
template <typename T>

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL