From 1b7372f2fb55f704b885e1097e2ec0381068c855 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 25 Jul 2017 12:20:40 +0200 Subject: [PATCH] src: replace ASSERT with CHECK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds always have asserts enabled so there is no point distinguishing between debug-only checks and run-time checks. Replace calls to ASSERT and friends with their CHECK counterparts. Fixes: https://github.com/nodejs/node/issues/14461 PR-URL: https://github.com/nodejs/node/pull/14474 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Michaël Zasso Reviewed-By: Nikolai Vavilov Reviewed-By: XadillaX --- node.gyp | 11 +---------- src/env-inl.h | 4 ++-- src/inspector_socket.cc | 12 ++++++------ src/node_buffer.cc | 18 +++++++++--------- src/node_crypto.cc | 2 +- src/node_crypto.h | 2 +- src/string_bytes.cc | 2 +- src/string_search.h | 10 +++++----- src/util.h | 15 +-------------- test/cctest/test_inspector_socket.cc | 2 +- test/cctest/test_inspector_socket_server.cc | 16 ++++++++-------- 11 files changed, 36 insertions(+), 58 deletions(-) diff --git a/node.gyp b/node.gyp index 31b4191690cda2..1650f1598bf02a 100644 --- a/node.gyp +++ b/node.gyp @@ -639,16 +639,7 @@ '<(OBJ_TRACING_PATH)<(OBJ_SEPARATOR)trace_event.<(OBJ_SUFFIX)', ], - 'defines': [ - # gtest's ASSERT macros conflict with our own. - 'GTEST_DONT_DEFINE_ASSERT_EQ=1', - 'GTEST_DONT_DEFINE_ASSERT_GE=1', - 'GTEST_DONT_DEFINE_ASSERT_GT=1', - 'GTEST_DONT_DEFINE_ASSERT_LE=1', - 'GTEST_DONT_DEFINE_ASSERT_LT=1', - 'GTEST_DONT_DEFINE_ASSERT_NE=1', - 'NODE_WANT_INTERNALS=1', - ], + 'defines': [ 'NODE_WANT_INTERNALS=1' ], 'sources': [ 'test/cctest/test_base64.cc', diff --git a/src/env-inl.h b/src/env-inl.h index cf7304c98dba76..e160fe27d8d5fc 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -272,14 +272,14 @@ inline Environment* Environment::GetCurrent(v8::Local context) { inline Environment* Environment::GetCurrent( const v8::FunctionCallbackInfo& info) { - ASSERT(info.Data()->IsExternal()); + CHECK(info.Data()->IsExternal()); return static_cast(info.Data().As()->Value()); } template inline Environment* Environment::GetCurrent( const v8::PropertyCallbackInfo& info) { - ASSERT(info.Data()->IsExternal()); + CHECK(info.Data()->IsExternal()); // XXX(bnoordhuis) Work around a g++ 4.9.2 template type inferrer bug // when the expression is written as info.Data().As(). v8::Local data = info.Data(); diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc index 85984b7fa11a05..2d5054bb802d95 100644 --- a/src/inspector_socket.cc +++ b/src/inspector_socket.cc @@ -157,7 +157,7 @@ static std::vector encode_frame_hybi17(const char* message, } frame.insert(frame.end(), extended_payload_length, extended_payload_length + 8); - ASSERT_EQ(0, remaining); + CHECK_EQ(0, remaining); } frame.insert(frame.end(), message, message + data_length); return frame; @@ -361,8 +361,8 @@ static void websockets_data_cb(uv_stream_t* stream, ssize_t nread, int inspector_read_start(InspectorSocket* inspector, uv_alloc_cb alloc_cb, uv_read_cb read_cb) { - ASSERT(inspector->ws_mode); - ASSERT(!inspector->shutting_down || read_cb == nullptr); + CHECK(inspector->ws_mode); + CHECK(!inspector->shutting_down || read_cb == nullptr); inspector->ws_state->close_sent = false; inspector->ws_state->alloc_cb = alloc_cb; inspector->ws_state->read_cb = read_cb; @@ -561,7 +561,7 @@ static void init_handshake(InspectorSocket* socket) { int inspector_accept(uv_stream_t* server, InspectorSocket* socket, handshake_cb callback) { - ASSERT_NE(callback, nullptr); + CHECK_NE(callback, nullptr); CHECK_EQ(socket->http_parsing_state, nullptr); socket->http_parsing_state = new http_parsing_state_s(); @@ -597,8 +597,8 @@ void inspector_close(InspectorSocket* inspector, inspector_cb callback) { // libuv throws assertions when closing stream that's already closed - we // need to do the same. - ASSERT(!uv_is_closing(reinterpret_cast(&inspector->tcp))); - ASSERT(!inspector->shutting_down); + CHECK(!uv_is_closing(reinterpret_cast(&inspector->tcp))); + CHECK(!inspector->shutting_down); inspector->shutting_down = true; inspector->ws_state->close_cb = callback; if (inspector->connection_eof) { diff --git a/src/node_buffer.cc b/src/node_buffer.cc index b3f5793f892b9d..dc67844553f64e 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -953,9 +953,9 @@ int64_t IndexOfOffset(size_t length, } void IndexOfString(const FunctionCallbackInfo& args) { - ASSERT(args[1]->IsString()); - ASSERT(args[2]->IsNumber()); - ASSERT(args[4]->IsBoolean()); + CHECK(args[1]->IsString()); + CHECK(args[2]->IsNumber()); + CHECK(args[4]->IsBoolean()); enum encoding enc = ParseEncoding(args.GetIsolate(), args[3], @@ -1069,9 +1069,9 @@ void IndexOfString(const FunctionCallbackInfo& args) { } void IndexOfBuffer(const FunctionCallbackInfo& args) { - ASSERT(args[1]->IsObject()); - ASSERT(args[2]->IsNumber()); - ASSERT(args[4]->IsBoolean()); + CHECK(args[1]->IsObject()); + CHECK(args[2]->IsNumber()); + CHECK(args[4]->IsBoolean()); enum encoding enc = ParseEncoding(args.GetIsolate(), args[3], @@ -1143,9 +1143,9 @@ void IndexOfBuffer(const FunctionCallbackInfo& args) { } void IndexOfNumber(const FunctionCallbackInfo& args) { - ASSERT(args[1]->IsNumber()); - ASSERT(args[2]->IsNumber()); - ASSERT(args[3]->IsBoolean()); + CHECK(args[1]->IsNumber()); + CHECK(args[2]->IsNumber()); + CHECK(args[3]->IsBoolean()); THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[0]); SPREAD_BUFFER_ARG(args[0], ts_obj); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 84f006b00f67eb..d03d8ad2f2ce8d 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5193,7 +5193,7 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo& args) { bool ECDH::IsKeyValidForCurve(const BIGNUM* private_key) { - ASSERT_NE(group_, nullptr); + CHECK_NE(group_, nullptr); CHECK_NE(private_key, nullptr); // Private keys must be in the range [1, n-1]. // Ref: Section 3.2.1 - http://www.secg.org/sec1-v2.pdf diff --git a/src/node_crypto.h b/src/node_crypto.h index 1d823bcb359a6a..b406fb8aa6867d 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -718,7 +718,7 @@ class ECDH : public BaseObject { key_(key), group_(EC_KEY_get0_group(key_)) { MakeWeak(this); - ASSERT_NE(group_, nullptr); + CHECK_NE(group_, nullptr); } static void New(const v8::FunctionCallbackInfo& args); diff --git a/src/string_bytes.cc b/src/string_bytes.cc index cbf30afc7af5dd..5aa3b8ca770192 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -321,7 +321,7 @@ size_t StringBytes::WriteUCS2(char* buf, uint16_t* aligned_dst = reinterpret_cast(buf + sizeof(*dst) - alignment); - ASSERT_EQ(reinterpret_cast(aligned_dst) % sizeof(*dst), 0); + CHECK_EQ(reinterpret_cast(aligned_dst) % sizeof(*dst), 0); // Write all but the last char nchars = str->Write(aligned_dst, 0, max_chars - 1, flags); diff --git a/src/string_search.h b/src/string_search.h index 6040888110c2ec..dfdb8e9a160460 100644 --- a/src/string_search.h +++ b/src/string_search.h @@ -28,7 +28,7 @@ class Vector { public: Vector(T* data, size_t length, bool isForward) : start_(data), length_(length), is_forward_(isForward) { - ASSERT(length > 0 && data != nullptr); + CHECK(length > 0 && data != nullptr); } // Returns the start of the memory range. @@ -44,7 +44,7 @@ class Vector { // Access individual vector elements - checks bounds in debug mode. T& operator[](size_t index) const { - ASSERT(index < length_); + CHECK(index < length_); return start_[is_forward_ ? index : (length_ - index - 1)]; } @@ -342,7 +342,7 @@ size_t StringSearch::LinearSearch( i = FindFirstCharacter(pattern, subject, i); if (i == subject.length()) return subject.length(); - ASSERT_LE(i, n); + CHECK_LE(i, n); bool matches = true; for (size_t j = 1; j < pattern_length; j++) { @@ -591,7 +591,7 @@ size_t StringSearch::InitialSearch( i = FindFirstCharacter(pattern, subject, i); if (i == subject.length()) return subject.length(); - ASSERT_LE(i, n); + CHECK_LE(i, n); size_t j = 1; do { if (pattern[j] != subject[i + j]) { @@ -644,7 +644,7 @@ size_t SearchString(const Char* haystack, needle, needle_length, is_forward); Vector v_haystack = Vector( haystack, haystack_length, is_forward); - ASSERT(haystack_length >= needle_length); + CHECK(haystack_length >= needle_length); size_t diff = haystack_length - needle_length; size_t relative_start_index; if (is_forward) { diff --git a/src/util.h b/src/util.h index 175db0c82cb725..1272de1893294c 100644 --- a/src/util.h +++ b/src/util.h @@ -77,7 +77,7 @@ void LowMemoryNotification(); #endif // The slightly odd function signature for Assert() is to ease -// instruction cache pressure in calls from ASSERT and CHECK. +// instruction cache pressure in calls from CHECK. NO_RETURN void Abort(); NO_RETURN void Assert(const char* const (*args)[4]); void DumpBacktrace(FILE* fp); @@ -124,19 +124,6 @@ template using remove_reference = std::remove_reference; } \ } while (0) -#ifdef NDEBUG -#define ASSERT(expr) -#else -#define ASSERT(expr) CHECK(expr) -#endif - -#define ASSERT_EQ(a, b) ASSERT((a) == (b)) -#define ASSERT_GE(a, b) ASSERT((a) >= (b)) -#define ASSERT_GT(a, b) ASSERT((a) > (b)) -#define ASSERT_LE(a, b) ASSERT((a) <= (b)) -#define ASSERT_LT(a, b) ASSERT((a) < (b)) -#define ASSERT_NE(a, b) ASSERT((a) != (b)) - #define CHECK_EQ(a, b) CHECK((a) == (b)) #define CHECK_GE(a, b) CHECK((a) >= (b)) #define CHECK_GT(a, b) CHECK((a) > (b)) diff --git a/test/cctest/test_inspector_socket.cc b/test/cctest/test_inspector_socket.cc index 741c7b189c2f38..d1f5b4a98ac37a 100644 --- a/test/cctest/test_inspector_socket.cc +++ b/test/cctest/test_inspector_socket.cc @@ -140,7 +140,7 @@ static void check_data_cb(read_expects* expectation, ssize_t nread, EXPECT_TRUE(nread >= 0 && nread != UV_EOF); ssize_t i; char c, actual; - ASSERT_GT(expectation->expected_len, 0); + CHECK_GT(expectation->expected_len, 0); for (i = 0; i < nread && expectation->pos <= expectation->expected_len; i++) { c = expectation->expected[expectation->pos++]; actual = buf->base[i]; diff --git a/test/cctest/test_inspector_socket_server.cc b/test/cctest/test_inspector_socket_server.cc index f9814274d0f91e..83512611824a0d 100644 --- a/test/cctest/test_inspector_socket_server.cc +++ b/test/cctest/test_inspector_socket_server.cc @@ -108,12 +108,12 @@ class TestInspectorServerDelegate : public SocketServerDelegate { } void MessageReceived(int session_id, const std::string& message) override { - ASSERT_EQ(session_id_, session_id); + CHECK_EQ(session_id_, session_id); buffer_.insert(buffer_.end(), message.begin(), message.end()); } void EndSession(int session_id) override { - ASSERT_EQ(session_id_, session_id); + CHECK_EQ(session_id_, session_id); disconnected++; } @@ -178,9 +178,9 @@ class SocketWrapper { } else { err = uv_ip4_addr(host.c_str(), port, &addr.v4); } - ASSERT_EQ(0, err); + CHECK_EQ(0, err); err = uv_tcp_connect(&connect_, &socket_, &addr.generic, Connected_); - ASSERT_EQ(0, err); + CHECK_EQ(0, err); SPIN_WHILE(!connected_) uv_read_start(reinterpret_cast(&socket_), AllocCallback, ReadCallback); @@ -195,11 +195,11 @@ class SocketWrapper { uv_tcp_init(loop_, &socket_); sockaddr_in addr; int err = uv_ip4_addr(host.c_str(), port, &addr); - ASSERT_EQ(0, err); + CHECK_EQ(0, err); err = uv_tcp_connect(&connect_, &socket_, reinterpret_cast(&addr), ConnectionMustFail_); - ASSERT_EQ(0, err); + CHECK_EQ(0, err); SPIN_WHILE(!connection_failed_) uv_read_start(reinterpret_cast(&socket_), AllocCallback, ReadCallback); @@ -244,7 +244,7 @@ class SocketWrapper { sending_ = true; int err = uv_write(&write_, reinterpret_cast(&socket_), buf, 1, WriteDone_); - ASSERT_EQ(err, 0); + CHECK_EQ(err, 0); SPIN_WHILE(sending_); } @@ -289,7 +289,7 @@ class SocketWrapper { delete[] buf->base; } static void WriteDone_(uv_write_t* req, int err) { - ASSERT_EQ(0, err); + CHECK_EQ(0, err); SocketWrapper* wrapper = node::ContainerOf(&SocketWrapper::write_, req); ASSERT_TRUE(wrapper->sending_);