diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index d7cde6e6..c96fead6 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -139,7 +139,8 @@ jobs: - name: Build Native Windows 32 if: ${{ matrix.os == 'windows-2019' && matrix.node_arch == 'ia32' }} run: - node ./node_modules/@aminya/cmake-ts/build/main.js named-configs windows-x86 + node ./node_modules/@aminya/cmake-ts/build/main.js named-configs + windows-x86 - name: Use Node 20 if: ${{ !matrix.docker }} diff --git a/.mocharc.js b/.mocharc.js index 602e4900..1411e927 100644 --- a/.mocharc.js +++ b/.mocharc.js @@ -8,6 +8,8 @@ const config = { "v8-expose-gc": true, exit: true, parallel: true, + timeout: 10000, + retries: 3, } module.exports = config diff --git a/CMakeLists.txt b/CMakeLists.txt index 710b76b9..90cfaaf0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -102,7 +102,12 @@ project_options(ENABLE_CACHE ENABLE_COMPILE_COMMANDS_SYMLINK) file(GLOB_RECURSE SOURCES "./src/*.cc") add_library(addon SHARED ${SOURCES}) -target_link_libraries(addon PRIVATE project_options) # project_warnings +if(CMAKE_CXX_COMPILER_ID STREQUAL GNU + OR CMAKE_CXX_COMPILER_ID STREQUAL Clang + OR CMAKE_CXX_COMPILER_ID STREQUAL AppleClang) + target_compile_options(project_warnings INTERFACE "-Wno-shadow") +endif() +target_link_libraries(addon PRIVATE project_options project_warnings) if(ZMQ_DRAFT) target_compile_definitions(addon PRIVATE ZMQ_BUILD_DRAFT_API) diff --git a/package.json b/package.json index 8549e6a7..1c74d845 100644 --- a/package.json +++ b/package.json @@ -95,7 +95,7 @@ "build.doc": "typedoc --options ./typedoc.json && minify-all -s docs-unminified -d docs --jsCompressor terser && shx rm -rf docs-unminified", "deploy.doc": "run-s build.doc && gh-pages --dist \"./docs\"", "build.native": "cmake-ts nativeonly", - "build.native.debug": "cmake-ts nativeonly", + "build.native.debug": "cmake-ts dev-os-only", "build": "run-p build.js build.native", "build.debug": "run-s build.js build.native.debug", "test": "run-s clean.temp build && mocha", @@ -107,7 +107,7 @@ "test.electron.renderer": "run-s build && electron-mocha --renderer", "lint-test.eslint": "eslint ./**/*.{ts,tsx,js,jsx,cjs,mjs,json,yaml} --no-error-on-unmatched-pattern --cache --cache-location ./.cache/eslint/", "lint.eslint": "pnpm run lint-test.eslint --fix", - "lint.clang-tidy": "git ls-files --exclude-standard | grep -E '\\.(cpp|hpp|c|cc|cxx|hxx|h|ixx)$' | xargs -n 1 -P $(nproc) clang-tidy --fix ", + "lint.clang-tidy": "git ls-files --exclude-standard | grep -E '\\.(cpp|hpp|c|cc|cxx|hxx|h|ixx)$' | xargs -n 1 -P $(nproc) clang-tidy", "lint": "run-p format lint.eslint format", "lint-test": "run-s lint-test.eslint", "bench": "node --expose-gc test/bench", @@ -124,6 +124,15 @@ "runtime": "node", "runtimeVersion": "12.22.12" }, + { + "name": "linux-x64-dev", + "dev": true, + "buildType": "Debug", + "os": "linux", + "arch": "x64", + "runtime": "node", + "runtimeVersion": "12.22.12" + }, { "name": "windows-x64", "os": "win32", diff --git a/src/closable.h b/src/closable.h index 891ebf7f..bf9eb403 100644 --- a/src/closable.h +++ b/src/closable.h @@ -4,6 +4,13 @@ up ZMQ resources at agent exit. */ namespace zmq { struct Closable { + Closable() = default; + Closable(const Closable&) = default; + Closable(Closable&&) = default; + Closable& operator=(const Closable&) = default; + Closable& operator=(Closable&&) = default; + virtual ~Closable() = default; + virtual void Close() = 0; }; -} +} // namespace zmq diff --git a/src/context.cc b/src/context.cc index ba6295ee..91d1970a 100644 --- a/src/context.cc +++ b/src/context.cc @@ -13,17 +13,21 @@ Context::Context(const Napi::CallbackInfo& info) /* If this module has no global context, then create one with a process wide context pointer that is shared between threads/agents. */ if (module.GlobalContext.IsEmpty()) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return; + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return; + } /* Just use the same shared global context pointer. Contexts are threadsafe anyway. */ context = module.Global().SharedContext; } else { - Arg::Validator args{ + Arg::Validator const args{ Arg::Optional("Options must be an object"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } context = zmq_ctx_new(); } @@ -62,7 +66,7 @@ void Context::Close() { termination may block depending on ZMQ_BLOCKY/ZMQ_LINGER. This should definitely be avoided during GC and may only be acceptable at process exit. */ - auto err = zmq_ctx_shutdown(context); + [[maybe_unused]] auto err = zmq_ctx_shutdown(context); assert(err == 0); /* Pass the ZMQ context on to terminator for cleanup at exit. */ @@ -76,35 +80,39 @@ void Context::Close() { template <> Napi::Value Context::GetCtxOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), }; - if (args.ThrowIfInvalid(info)) return Env().Undefined(); + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - uint32_t option = info[0].As(); + const auto option = info[0].As(); - int32_t value = zmq_ctx_get(context, option); + int32_t const value = zmq_ctx_get(context, option); if (value < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); return Env().Undefined(); } - return Napi::Boolean::New(Env(), value); + return Napi::Boolean::New(Env(), value != 0); } template <> void Context::SetCtxOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), Arg::Required("Option value must be a boolean"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - uint32_t option = info[0].As(); + const auto option = info[0].As(); - int32_t value = info[1].As(); + int32_t const value = static_cast(info[1].As()); if (zmq_ctx_set(context, option, value) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); return; @@ -113,13 +121,15 @@ void Context::SetCtxOpt(const Napi::CallbackInfo& info) { template Napi::Value Context::GetCtxOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), }; - if (args.ThrowIfInvalid(info)) return Env().Undefined(); + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - uint32_t option = info[0].As(); + const auto option = info[0].As(); T value = zmq_ctx_get(context, option); if (value < 0) { @@ -132,14 +142,16 @@ Napi::Value Context::GetCtxOpt(const Napi::CallbackInfo& info) { template void Context::SetCtxOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), Arg::Required("Option value must be a number"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - uint32_t option = info[0].As(); + const auto option = info[0].As(); T value = info[1].As(); if (zmq_ctx_set(context, option, value) < 0) { @@ -166,4 +178,4 @@ void Context::Initialize(Module& module, Napi::Object& exports) { module.Context = Napi::Persistent(constructor); exports.Set("Context", constructor); } -} +} // namespace zmq diff --git a/src/context.h b/src/context.h index f75c138d..e834925b 100644 --- a/src/context.h +++ b/src/context.h @@ -12,10 +12,12 @@ class Context : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Context(const Napi::CallbackInfo& info); - virtual ~Context(); + Context(const Context&) = delete; + Context& operator=(const Context&) = delete; Context(Context&&) = delete; Context& operator=(Context&&) = delete; + ~Context() override; void Close() override; @@ -34,7 +36,7 @@ class Context : public Napi::ObjectWrap, public Closable { friend class Observer; friend class Proxy; }; -} +} // namespace zmq -static_assert(!std::is_copy_constructible::value, "not copyable"); -static_assert(!std::is_move_constructible::value, "not movable"); +static_assert(!std::is_copy_constructible_v, "not copyable"); +static_assert(!std::is_move_constructible_v, "not movable"); diff --git a/src/incoming_msg.cc b/src/incoming_msg.cc index c2fa967c..0d105405 100644 --- a/src/incoming_msg.cc +++ b/src/incoming_msg.cc @@ -2,6 +2,7 @@ #include "./incoming_msg.h" #include +#include #include "util/electron_helper.h" #include "util/error.h" @@ -26,11 +27,11 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { return env.Undefined(); } } - auto data = reinterpret_cast(zmq_msg_data(*ref)); - auto length = zmq_msg_size(*ref); + auto* data = reinterpret_cast(zmq_msg_data(ref->get())); + auto length = zmq_msg_size(ref->get()); if (noElectronMemoryCage) { - static auto constexpr zero_copy_threshold = 1 << 7; + static auto constexpr zero_copy_threshold = 1U << 7U; if (length > zero_copy_threshold) { /* Reuse existing buffer for external storage. This avoids copying but does include an overhead in having to call a finalizer when the @@ -38,32 +39,34 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { moved = true; /* Put appropriate GC pressure according to the size of the buffer. */ - Napi::MemoryManagement::AdjustExternalMemory(env, length); + Napi::MemoryManagement::AdjustExternalMemory( + env, static_cast(length)); - auto release = [](const Napi::Env& env, uint8_t*, Reference* ref) { - ptrdiff_t length = zmq_msg_size(*ref); + const auto release = [](const Napi::Env& env, uint8_t*, Reference* ref) { + const auto length = static_cast(zmq_msg_size(ref->get())); Napi::MemoryManagement::AdjustExternalMemory(env, -length); delete ref; }; - return Napi::Buffer::New(env, data, length, release, ref); + return Napi::Buffer::New(env, data, length, release, ref) + .As(); } } if (length > 0) { - return Napi::Buffer::Copy(env, data, length); + return Napi::Buffer::Copy(env, data, length).As(); } - return Napi::Buffer::New(env, 0); + return Napi::Buffer::New(env, 0).As(); } IncomingMsg::Reference::Reference() { - auto err = zmq_msg_init(&msg); + [[maybe_unused]] auto err = zmq_msg_init(&msg); assert(err == 0); } IncomingMsg::Reference::~Reference() { - auto err = zmq_msg_close(&msg); + [[maybe_unused]] auto err = zmq_msg_close(&msg); assert(err == 0); } -} +} // namespace zmq diff --git a/src/incoming_msg.h b/src/incoming_msg.h index ba5c46c8..6d1252d9 100644 --- a/src/incoming_msg.h +++ b/src/incoming_msg.h @@ -12,22 +12,28 @@ class IncomingMsg { IncomingMsg(const IncomingMsg&) = delete; IncomingMsg& operator=(const IncomingMsg&) = delete; + IncomingMsg(IncomingMsg&&) = delete; + IncomingMsg& operator=(IncomingMsg&&) = delete; Napi::Value IntoBuffer(const Napi::Env& env); - inline operator zmq_msg_t*() { - return *ref; + zmq_msg_t* get() { + return ref->get(); } private: class Reference { - zmq_msg_t msg; + zmq_msg_t msg{}; public: Reference(); + Reference(const Reference&) = delete; + Reference(Reference&&) = delete; + Reference& operator=(const Reference&) = delete; + Reference& operator=(Reference&&) = delete; ~Reference(); - inline operator zmq_msg_t*() { + zmq_msg_t* get() { return &msg; } }; @@ -35,7 +41,7 @@ class IncomingMsg { Reference* ref = nullptr; bool moved = false; }; -} +} // namespace zmq -static_assert(!std::is_copy_constructible::value, "not copyable"); -static_assert(!std::is_move_constructible::value, "not movable"); +static_assert(!std::is_copy_constructible_v, "not copyable"); +static_assert(!std::is_move_constructible_v, "not movable"); diff --git a/src/inline.h b/src/inline.h index bb8a7b0d..95031f6f 100644 --- a/src/inline.h +++ b/src/inline.h @@ -1,7 +1,7 @@ #pragma once -#ifdef _MSC_VER -#define force_inline inline __forceinline +#if defined(_MSC_VER) && !defined(__clang__) +#define force_inline __forceinline #else #define force_inline inline __attribute__((always_inline)) #endif diff --git a/src/module.cc b/src/module.cc index 1904eb19..1c854f53 100644 --- a/src/module.cc +++ b/src/module.cc @@ -1,4 +1,6 @@ +#include + #include "./module.h" #include "./context.h" @@ -6,24 +8,27 @@ #include "./outgoing_msg.h" #include "./proxy.h" #include "./socket.h" +#include "./zmq_inc.h" #include "util/error.h" -#include "util/to_string.h" namespace zmq { -static inline Napi::String Version(const Napi::Env& env) { - int32_t major, minor, patch; +Napi::String Version(const Napi::Env& env) { + int32_t major = 0; + int32_t minor = 0; + int32_t patch = 0; zmq_version(&major, &minor, &patch); - return Napi::String::New( - env, to_string(major) + "." + to_string(minor) + "." + to_string(patch)); + return Napi::String::New(env, + std::to_string(major) + "." + std::to_string(minor) + "." + + std::to_string(patch)); } -static inline Napi::Object Capabilities(const Napi::Env& env) { +Napi::Object Capabilities(const Napi::Env& env) { auto result = Napi::Object::New(env); #ifdef ZMQ_HAS_CAPABILITIES static auto options = {"ipc", "pgm", "tipc", "norm", "curve", "gssapi", "draft"}; - for (auto& option : options) { + for (const auto& option : options) { result.Set(option, static_cast(zmq_has(option))); } @@ -54,22 +59,24 @@ static inline Napi::Object Capabilities(const Napi::Env& env) { return result; } -static inline Napi::Value CurveKeyPair(const Napi::CallbackInfo& info) { - char public_key[41]; - char secret_key[41]; - if (zmq_curve_keypair(public_key, secret_key) < 0) { +Napi::Value CurveKeyPair(const Napi::CallbackInfo& info) { + static constexpr auto max_key_length = 41; + + std::array public_key{}; + std::array secret_key{}; + + if (zmq_curve_keypair(public_key.data(), secret_key.data()) < 0) { ErrnoException(info.Env(), zmq_errno()).ThrowAsJavaScriptException(); return info.Env().Undefined(); } auto result = Napi::Object::New(info.Env()); - result["publicKey"] = Napi::String::New(info.Env(), public_key); - result["secretKey"] = Napi::String::New(info.Env(), secret_key); + result["publicKey"] = Napi::String::New(info.Env(), public_key.data()); + result["secretKey"] = Napi::String::New(info.Env(), secret_key.data()); return result; } -Module::Global::Global() { - SharedContext = zmq_ctx_new(); +Module::Global::Global() : SharedContext(zmq_ctx_new()) { assert(SharedContext != nullptr); ContextTerminator.Add(SharedContext); @@ -82,7 +89,7 @@ Module::Global::Shared Module::Global::Instance() { static std::mutex lock; static std::weak_ptr shared; - std::lock_guard guard(lock); + std::lock_guard const guard(lock); /* Get an existing instance from the weak reference, if possible. */ if (auto instance = shared.lock()) { @@ -108,15 +115,15 @@ Module::Module(Napi::Object exports) : MsgTrash(exports.Env()) { Proxy::Initialize(*this, exports); #endif } -} +} // namespace zmq /* This initializer can be called in multiple contexts, like worker threads. */ NAPI_MODULE_INIT(/* env, exports */) { - auto module = new zmq::Module(Napi::Object(env, exports)); + auto* module = new zmq::Module(Napi::Object(env, exports)); auto terminate = [](void* data) { delete reinterpret_cast(data); }; /* Tear down the module class when the env/agent/thread is closed.*/ - auto status = napi_add_env_cleanup_hook(env, terminate, module); + [[maybe_unused]] auto status = napi_add_env_cleanup_hook(env, terminate, module); assert(status == napi_ok); return exports; } diff --git a/src/module.h b/src/module.h index f60080e2..5e3a70a7 100644 --- a/src/module.h +++ b/src/module.h @@ -18,16 +18,16 @@ struct Terminator { assert(context != nullptr); #ifdef ZMQ_BLOCKY - bool blocky = zmq_ctx_get(context, ZMQ_BLOCKY); + const bool blocky = zmq_ctx_get(context, ZMQ_BLOCKY) != 0; #else /* If the option cannot be set, don't suggest to set it. */ - bool blocky = false; + const bool blocky = false; #endif /* Start termination asynchronously so we can detect if it takes long and should warn the user about this default blocking behaviour. */ auto terminate = std::async(std::launch::async, [&] { - auto err = zmq_ctx_term(context); + [[maybe_unused]] auto err = zmq_ctx_term(context); assert(err == 0); }); @@ -35,7 +35,7 @@ struct Terminator { if (terminate.wait_for(500ms) == std::future_status::timeout) { /* We can't use process.emitWarning, because the Node.js runtime has already shut down. So we mimic it instead. */ - fprintf(stderr, + (void)fprintf(stderr, "(node:%d) WARNING: Waiting for queued ZeroMQ messages to be " "delivered.%s\n", uv_os_getpid(), @@ -69,7 +69,7 @@ class Module { public: explicit Module(Napi::Object exports); - inline class Global& Global() { + class Global& Global() { return *global; } @@ -103,7 +103,7 @@ class Module { Napi::FunctionReference Observer; Napi::FunctionReference Proxy; }; -} +} // namespace zmq -static_assert(!std::is_copy_constructible::value, "not copyable"); -static_assert(!std::is_move_constructible::value, "not movable"); +static_assert(!std::is_copy_constructible_v, "not copyable"); +static_assert(!std::is_move_constructible_v, "not movable"); diff --git a/src/observer.cc b/src/observer.cc index 5fb728c9..d615f157 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -1,6 +1,8 @@ #include "./observer.h" +#include + #include "./context.h" #include "./module.h" #include "./socket.h" @@ -10,7 +12,7 @@ #include "util/take.h" namespace zmq { -static inline constexpr const char* EventName(uint32_t val) { +constexpr const char* EventName(uint32_t val) { switch (val) { case ZMQ_EVENT_CONNECTED: return "connect"; @@ -74,7 +76,8 @@ static inline constexpr const char* EventName(uint32_t val) { } #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_AUTH -static inline constexpr const char* AuthError(uint32_t val) { +constexpr const char* AuthError(uint32_t val) { + // NOLINTBEGIN(*-magic-numbers) switch (val) { case 300: return "Temporary error"; @@ -86,11 +89,13 @@ static inline constexpr const char* AuthError(uint32_t val) { /* Fallback if the auth error was unknown, which should not happen. */ return "Unknown error"; } + // NOLINTEND(*-magic-numbers) } #endif #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL -static inline std::pair ProtoError(uint32_t val) { +std::pair ProtoError(uint32_t val) { +// NOLINTNEXTLINE(*-macro-usage) #define PROTO_ERROR_CASE(_prefix, _err) \ case ZMQ_PROTOCOL_ERROR_##_prefix##_##_err: \ return std::make_pair(#_prefix " protocol error", "ERR_" #_prefix "_" #_err); @@ -126,60 +131,62 @@ static inline std::pair ProtoError(uint32_t val) { Observer::Observer(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info), async_context(Env(), "Observer"), poller(*this), module(*reinterpret_cast(info.Data())) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Socket must be a socket object"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - auto target = Socket::Unwrap(info[0].As()); - if (Env().IsExceptionPending()) return; + auto* target = Socket::Unwrap(info[0].As()); + if (Env().IsExceptionPending()) { + return; + } /* Use `this` pointer as unique identifier for monitoring socket. */ auto address = std::string("inproc://zmq.monitor.") - + to_string(reinterpret_cast(this)); + + std::to_string(reinterpret_cast(this)); if (zmq_socket_monitor(target->socket, address.c_str(), ZMQ_EVENT_ALL) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); return; } - auto context = Context::Unwrap(target->context_ref.Value()); + auto* context = Context::Unwrap(target->context_ref.Value()); socket = zmq_socket(context->context, ZMQ_PAIR); if (socket == nullptr) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); return; } - uv_os_sock_t fd; - size_t length = sizeof(fd); + uv_os_sock_t file_descriptor = 0; + size_t length = sizeof(file_descriptor); + + const auto error = [this]() { + [[maybe_unused]] auto err = zmq_close(socket); + assert(err == 0); + + socket = nullptr; + }; if (zmq_connect(socket, address.c_str()) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); - goto error; + error(); } - if (zmq_getsockopt(socket, ZMQ_FD, &fd, &length) < 0) { + if (zmq_getsockopt(socket, ZMQ_FD, &file_descriptor, &length) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); - goto error; + error(); } - if (poller.Initialize(Env(), fd) < 0) { + if (poller.Initialize(Env(), file_descriptor) < 0) { ErrnoException(Env(), errno).ThrowAsJavaScriptException(); - goto error; + error(); } /* Initialization was successful, register the observer for cleanup. */ module.ObjectReaper.Add(this); - - return; - -error: - auto err = zmq_close(socket); - assert(err == 0); - - socket = nullptr; - return; } Observer::~Observer() { @@ -196,25 +203,27 @@ bool Observer::ValidateOpen() const { } bool Observer::HasEvents() const { - int32_t events; + uint32_t events = 0; size_t events_size = sizeof(events); while (zmq_getsockopt(socket, ZMQ_EVENTS, &events, &events_size) < 0) { /* Ignore errors. */ - if (zmq_errno() != EINTR) return 0; + if (zmq_errno() != EINTR) { + return false; + } } - return events & ZMQ_POLLIN; + return (events & ZMQ_POLLIN) != 0; } void Observer::Close() { if (socket != nullptr) { module.ObjectReaper.Remove(this); - Napi::HandleScope scope(Env()); + Napi::HandleScope const scope(Env()); /* Close succeeds unless socket is invalid. */ - auto err = zmq_close(socket); + [[maybe_unused]] auto err = zmq_close(socket); assert(err == 0); /* Reset pointer to avoid double close. */ @@ -240,7 +249,7 @@ void Observer::Receive(const Napi::Promise::Deferred& res) { } } - auto data1 = static_cast(zmq_msg_data(&msg1)); + auto* data1 = static_cast(zmq_msg_data(&msg1)); auto event_id = *reinterpret_cast(data1); auto value = *reinterpret_cast(data1 + 2); zmq_msg_close(&msg1); @@ -254,7 +263,7 @@ void Observer::Receive(const Napi::Promise::Deferred& res) { } } - auto data2 = reinterpret_cast(zmq_msg_data(&msg2)); + auto* data2 = reinterpret_cast(zmq_msg_data(&msg2)); auto length = zmq_msg_size(&msg2); auto event = Napi::Object::New(Env()); @@ -278,7 +287,7 @@ void Observer::Receive(const Napi::Promise::Deferred& res) { #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL case ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL: #endif - event["error"] = ErrnoException(Env(), value).Value(); + event["error"] = ErrnoException(Env(), static_cast(value)).Value(); break; #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL @@ -300,21 +309,29 @@ void Observer::Receive(const Napi::Promise::Deferred& res) { Close(); break; } + default: + break; } res.Resolve(event); } void Observer::Close(const Napi::CallbackInfo& info) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return; + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return; + } Close(); } Napi::Value Observer::Receive(const Napi::CallbackInfo& info) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return Env().Undefined(); + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - if (!ValidateOpen()) return Env().Undefined(); + if (!ValidateOpen()) { + return Env().Undefined(); + } if (poller.Reading()) { ErrnoException(Env(), EBUSY, @@ -329,13 +346,12 @@ Napi::Value Observer::Receive(const Napi::CallbackInfo& info) { auto res = Napi::Promise::Deferred::New(Env()); Receive(res); return res.Promise(); - } else { - poller.PollReadable(0); - return poller.ReadPromise(); } + poller.PollReadable(0); + return poller.ReadPromise(); } -Napi::Value Observer::GetClosed(const Napi::CallbackInfo& info) { +Napi::Value Observer::GetClosed(const Napi::CallbackInfo& /*info*/) { return Napi::Boolean::New(Env(), socket == nullptr); } @@ -354,14 +370,14 @@ void Observer::Initialize(Module& module, Napi::Object& exports) { void Observer::Poller::ReadableCallback() { assert(read_deferred); - AsyncScope scope(socket.Env(), socket.async_context); - socket.Receive(take(read_deferred)); + AsyncScope const scope(socket.get().Env(), socket.get().async_context); + socket.get().Receive(take(read_deferred)); } Napi::Value Observer::Poller::ReadPromise() { assert(!read_deferred); - read_deferred = Napi::Promise::Deferred(socket.Env()); + read_deferred = Napi::Promise::Deferred(socket.get().Env()); return read_deferred->Promise(); } -} +} // namespace zmq diff --git a/src/observer.h b/src/observer.h index e902b5e3..2e0be4ff 100644 --- a/src/observer.h +++ b/src/observer.h @@ -2,6 +2,7 @@ #include +#include #include #include "./closable.h" @@ -16,7 +17,12 @@ class Observer : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Observer(const Napi::CallbackInfo& info); - virtual ~Observer(); + + Observer(const Observer&) = delete; + Observer(Observer&&) = delete; + Observer& operator=(const Observer&) = delete; + Observer& operator=(Observer&&) = delete; + ~Observer() override; void Close() override; @@ -27,34 +33,34 @@ class Observer : public Napi::ObjectWrap, public Closable { inline Napi::Value GetClosed(const Napi::CallbackInfo& info); private: - inline bool ValidateOpen() const; - bool HasEvents() const; + [[nodiscard]] inline bool ValidateOpen() const; + [[nodiscard]] bool HasEvents() const; force_inline void Receive(const Napi::Promise::Deferred& res); class Poller : public zmq::Poller { - Observer& socket; + std::reference_wrapper socket; std::optional read_deferred; public: - explicit Poller(Observer& observer) : socket(observer) {} + explicit Poller(std::reference_wrapper observer) : socket(observer) {} Napi::Value ReadPromise(); - inline bool Reading() const { + [[nodiscard]] bool Reading() const { return read_deferred.has_value(); } - inline bool ValidateReadable() const { - return socket.HasEvents(); + [[nodiscard]] bool ValidateReadable() const { + return socket.get().HasEvents(); } - inline bool ValidateWritable() const { + [[nodiscard]] static bool ValidateWritable() { return false; } void ReadableCallback(); - inline void WritableCallback() {} + void WritableCallback() {} }; Napi::AsyncContext async_context; @@ -65,7 +71,7 @@ class Observer : public Napi::ObjectWrap, public Closable { friend class Socket; }; -} +} // namespace zmq -static_assert(!std::is_copy_constructible::value, "not copyable"); -static_assert(!std::is_move_constructible::value, "not movable"); +static_assert(!std::is_copy_constructible_v, "not copyable"); +static_assert(!std::is_move_constructible_v, "not movable"); diff --git a/src/outgoing_msg.cc b/src/outgoing_msg.cc index f32a4619..75bf7355 100644 --- a/src/outgoing_msg.cc +++ b/src/outgoing_msg.cc @@ -1,12 +1,13 @@ #include "./outgoing_msg.h" +#include #include "./module.h" #include "util/error.h" namespace zmq { -OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { - static auto constexpr zero_copy_threshold = 1 << 7; +OutgoingMsg::OutgoingMsg(Napi::Value value, std::reference_wrapper module) { + static auto constexpr zero_copy_threshold = 1U << 7U; auto buffer_send = [&](uint8_t* data, size_t length) { /* Zero-copy heuristic. There's an overhead in releasing the buffer with an @@ -18,7 +19,7 @@ OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { message is sent by ZeroMQ on an *arbitrary* thread. It will add the reference to the global trash, which will schedule a callback on the main v8 thread in order to safely dispose of the reference. */ - auto ref = new Reference(value, module); + auto* ref = new Reference(value, module); auto recycle = [](void*, void* item) { static_cast(item)->Recycle(); }; @@ -44,7 +45,7 @@ OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { but once converted we do not have to copy a second time. */ auto string_send = [&](std::string* str) { auto length = str->size(); - auto data = const_cast(str->data()); + auto* data = str->data(); auto release = [](void*, void* str) { delete reinterpret_cast(str); @@ -81,6 +82,7 @@ OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { } /* Fall through */ + [[fallthrough]]; default: string_send(new std::string(value.ToString())); } @@ -88,19 +90,19 @@ OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { } OutgoingMsg::~OutgoingMsg() { - auto err = zmq_msg_close(&msg); + [[maybe_unused]] auto err = zmq_msg_close(&msg); assert(err == 0); } void OutgoingMsg::Reference::Recycle() { - module.MsgTrash.Add(this); + module.get().MsgTrash.Add(this); } OutgoingMsg::Parts::Parts(Napi::Value value, Module& module) { if (value.IsArray()) { /* Reverse insert parts into outgoing message list. */ auto arr = value.As(); - for (auto i = arr.Length(); i--;) { + for (auto i = arr.Length(); (i--) != 0U;) { parts.emplace_front(arr[i], module); } } else { @@ -157,4 +159,4 @@ bool OutgoingMsg::Parts::SetRoutingId(Napi::Value value) { } #endif -} +} // namespace zmq diff --git a/src/outgoing_msg.h b/src/outgoing_msg.h index a86d4373..9046e9cb 100644 --- a/src/outgoing_msg.h +++ b/src/outgoing_msg.h @@ -3,6 +3,7 @@ #include #include +#include #include "./zmq_inc.h" @@ -17,30 +18,32 @@ class OutgoingMsg { nor should we have to copy messages with the right STL containers. */ OutgoingMsg(const OutgoingMsg&) = delete; OutgoingMsg& operator=(const OutgoingMsg&) = delete; + OutgoingMsg(OutgoingMsg&&) = delete; + OutgoingMsg& operator=(OutgoingMsg&&) = delete; /* Outgoing message. Takes a string or buffer argument and releases the underlying V8 resources whenever the message is sent, or earlier if the message was copied (small buffers & strings). */ - explicit OutgoingMsg(Napi::Value value, Module& module); + explicit OutgoingMsg(Napi::Value value, std::reference_wrapper module); ~OutgoingMsg(); - inline operator zmq_msg_t*() { + zmq_msg_t* get() { return &msg; } private: class Reference { Napi::Reference persistent; - Module& module; + std::reference_wrapper module; public: - inline explicit Reference(Napi::Value val, Module& module) + explicit Reference(Napi::Value val, std::reference_wrapper module) : persistent(Napi::Persistent(val)), module(module) {} void Recycle(); }; - zmq_msg_t msg; + zmq_msg_t msg{}; friend class Module; }; @@ -51,14 +54,14 @@ class OutgoingMsg::Parts { std::forward_list parts; public: - inline Parts() {} + Parts() = default; explicit Parts(Napi::Value value, Module& module); - inline std::forward_list::iterator begin() { + std::forward_list::iterator begin() { return parts.begin(); } - inline std::forward_list::iterator end() { + std::forward_list::iterator end() { return parts.end(); } @@ -67,11 +70,11 @@ class OutgoingMsg::Parts { bool SetRoutingId(Napi::Value value); #endif - inline void Clear() { + void Clear() { parts.clear(); } }; -} +} // namespace zmq -static_assert(!std::is_copy_constructible::value, "not copyable"); -static_assert(!std::is_move_constructible::value, "not movable"); +static_assert(!std::is_copy_constructible_v, "not copyable"); +static_assert(!std::is_move_constructible_v, "not movable"); diff --git a/src/poller.h b/src/poller.h index 7f885f9c..cbeb8088 100644 --- a/src/poller.h +++ b/src/poller.h @@ -1,5 +1,8 @@ #pragma once +#include +#include + #include "util/uvhandle.h" #include "util/uvloop.h" @@ -13,39 +16,39 @@ class Poller { UvHandle readable_timer; UvHandle writable_timer; - int32_t events{0}; + uint32_t events{0}; std::function finalize = nullptr; public: /* Initialize the poller with the given file descriptor. FD should be ZMQ style edge-triggered, with READABLE state indicating that ANY event may be present on the corresponding ZMQ socket. */ - inline int32_t Initialize( - Napi::Env env, uv_os_sock_t& fd, std::function finalizer = nullptr) { - auto loop = UvLoop(env); + int32_t Initialize( + Napi::Env env, uv_os_sock_t& file_descriptor, std::function finalizer = nullptr) { + auto* loop = UvLoop(env); poll->data = this; - if (auto err = uv_poll_init_socket(loop, poll, fd); err != 0) { + if (auto err = uv_poll_init_socket(loop, poll.get(), file_descriptor); err != 0) { return err; } readable_timer->data = this; - if (auto err = uv_timer_init(loop, readable_timer); err != 0) { + if (auto err = uv_timer_init(loop, readable_timer.get()); err != 0) { return err; } writable_timer->data = this; - if (auto err = uv_timer_init(loop, writable_timer); err != 0) { + if (auto err = uv_timer_init(loop, writable_timer.get()); err != 0) { return err; } - finalize = finalizer; + finalize = std::move(finalizer); return 0; } /* Safely close and release all handles. This can be called before destruction to release resources early. */ - inline void Close() { + void Close() { /* Trigger watched events manually, which causes any pending operation to succeed or fail immediately. */ Trigger(events); @@ -58,45 +61,49 @@ class Poller { readable_timer.reset(); writable_timer.reset(); - if (finalize) finalize(); + if (finalize) { + finalize(); + } } /* Start polling for readable state, with the given timeout. */ - inline void PollReadable(int64_t timeout) { + void PollReadable(int64_t timeout) { assert((events & UV_READABLE) == 0); if (timeout > 0) { - auto err = uv_timer_start( - readable_timer, + [[maybe_unused]] auto err = uv_timer_start( + readable_timer.get(), [](uv_timer_t* timer) { + // NOLINTNEXTLINE(*-pro-type-reinterpret-cast) auto& poller = *reinterpret_cast(timer->data); poller.Trigger(UV_READABLE); }, - timeout, 0); + static_cast(timeout), 0); assert(err == 0); } - if (!events) { + if (events == 0) { /* Only start polling if we were not polling already. */ - auto err = uv_poll_start(poll, UV_READABLE, Callback); + [[maybe_unused]] auto err = uv_poll_start(poll.get(), UV_READABLE, Callback); assert(err == 0); } events |= UV_READABLE; } - inline void PollWritable(int64_t timeout) { + void PollWritable(int64_t timeout) { assert((events & UV_WRITABLE) == 0); if (timeout > 0) { - auto err = uv_timer_start( - writable_timer, + [[maybe_unused]] auto err = uv_timer_start( + writable_timer.get(), [](uv_timer_t* timer) { + // NOLINTNEXTLINE(*-pro-type-reinterpret-cast) auto& poller = *reinterpret_cast(timer->data); poller.Trigger(UV_WRITABLE); }, - timeout, 0); + static_cast(timeout), 0); assert(err == 0); } @@ -104,8 +111,8 @@ class Poller { /* Note: We poll for READS only! "ZMQ shall signal ANY pending events on the socket in an edge-triggered fashion by making the file descriptor become ready for READING." */ - if (!events) { - auto err = uv_poll_start(poll, UV_READABLE, Callback); + if (events == 0) { + [[maybe_unused]] auto err = uv_poll_start(poll.get(), UV_READABLE, Callback); assert(err == 0); } @@ -114,16 +121,16 @@ class Poller { /* Trigger any events that are ready. Use validation callbacks to see which events are actually available. */ - inline void TriggerReadable() { - if (events & UV_READABLE) { + void TriggerReadable() { + if ((events & UV_READABLE) != 0) { if (static_cast(this)->ValidateReadable()) { Trigger(UV_READABLE); } } } - inline void TriggerWritable() { - if (events & UV_WRITABLE) { + void TriggerWritable() { + if ((events & UV_WRITABLE) != 0) { if (static_cast(this)->ValidateWritable()) { Trigger(UV_WRITABLE); } @@ -134,21 +141,21 @@ class Poller { /* Trigger one or more specific events manually. No validation is performed, which means these will cause EAGAIN errors if no events were actually available. */ - inline void Trigger(int32_t triggered) { + void Trigger(uint32_t triggered) { events &= ~triggered; - if (!events) { - auto err = uv_poll_stop(poll); + if (events == 0) { + [[maybe_unused]] auto err = uv_poll_stop(poll.get()); assert(err == 0); } - if (triggered & UV_READABLE) { - auto err = uv_timer_stop(readable_timer); + if ((triggered & UV_READABLE) != 0) { + [[maybe_unused]] auto err = uv_timer_stop(readable_timer.get()); assert(err == 0); static_cast(this)->ReadableCallback(); } - if (triggered & UV_WRITABLE) { - auto err = uv_timer_stop(writable_timer); + if ((triggered & UV_WRITABLE) != 0) { + [[maybe_unused]] auto err = uv_timer_stop(writable_timer.get()); assert(err == 0); static_cast(this)->WritableCallback(); } @@ -157,12 +164,13 @@ class Poller { /* Callback is called when FD is set to a readable state. This is an edge trigger that should allow us to check for read AND write events. There is no guarantee that any events are available. */ - static void Callback(uv_poll_t* poll, int32_t status, int32_t events) { + static void Callback(uv_poll_t* poll, int32_t status, int32_t /*events*/) { if (status == 0) { + // NOLINTNEXTLINE(*-pro-type-reinterpret-cast) auto& poller = *reinterpret_cast(poll->data); poller.TriggerReadable(); poller.TriggerWritable(); } } }; -} +} // namespace zmq diff --git a/src/proxy.cc b/src/proxy.cc index 3f969cc3..520f2519 100644 --- a/src/proxy.cc +++ b/src/proxy.cc @@ -1,6 +1,8 @@ #include "./proxy.h" +#include + #include "./context.h" #include "./module.h" #include "./socket.h" @@ -22,34 +24,46 @@ struct ProxyContext { Proxy::Proxy(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info), async_context(Env(), "Proxy"), module(*reinterpret_cast(info.Data())) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Front-end must be a socket object"), Arg::Required("Back-end must be a socket object"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } front_ref.Reset(info[0].As(), 1); Socket::Unwrap(front_ref.Value()); - if (Env().IsExceptionPending()) return; + if (Env().IsExceptionPending()) { + return; + } back_ref.Reset(info[1].As(), 1); Socket::Unwrap(back_ref.Value()); - if (Env().IsExceptionPending()) return; + if (Env().IsExceptionPending()) { + return; + } } -Proxy::~Proxy() {} +Proxy::~Proxy() = default; void Proxy::Close() {} Napi::Value Proxy::Run(const Napi::CallbackInfo& info) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return Env().Undefined(); + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - auto front = Socket::Unwrap(front_ref.Value()); - if (Env().IsExceptionPending()) return Env().Undefined(); + auto* front = Socket::Unwrap(front_ref.Value()); + if (Env().IsExceptionPending()) { + return Env().Undefined(); + } - auto back = Socket::Unwrap(back_ref.Value()); - if (Env().IsExceptionPending()) return Env().Undefined(); + auto* back = Socket::Unwrap(back_ref.Value()); + if (Env().IsExceptionPending()) { + return Env().Undefined(); + } if (front->endpoints == 0) { ErrnoException(Env(), EINVAL, "Front-end socket must be bound or connected") @@ -63,8 +77,10 @@ Napi::Value Proxy::Run(const Napi::CallbackInfo& info) { return Env().Undefined(); } - auto context = Context::Unwrap(front->context_ref.Value()); - if (Env().IsExceptionPending()) return Env().Undefined(); + auto* context = Context::Unwrap(front->context_ref.Value()); + if (Env().IsExceptionPending()) { + return Env().Undefined(); + } control_sub = zmq_socket(context->context, ZMQ_DEALER); if (control_sub == nullptr) { @@ -80,7 +96,7 @@ Napi::Value Proxy::Run(const Napi::CallbackInfo& info) { /* Use `this` pointer as unique identifier for control socket. */ auto address = std::string("inproc://zmq.proxycontrol.") - + to_string(reinterpret_cast(this)); + + std::to_string(reinterpret_cast(this)); /* Connect publisher so we can start queueing control messages. */ if (zmq_connect(control_pub, address.c_str()) < 0) { @@ -94,40 +110,41 @@ Napi::Value Proxy::Run(const Napi::CallbackInfo& info) { auto res = Napi::Promise::Deferred::New(Env()); auto run_ctx = std::make_shared(std::move(address)); - auto front_ptr = front->socket; - auto back_ptr = back->socket; + auto* front_ptr = front->socket; + auto* back_ptr = back->socket; auto status = UvQueue( Env(), - [=]() { + [this, run_ctx, front_ptr, back_ptr]() { /* Don't access V8 internals here! Executed in worker thread. */ if (zmq_bind(control_sub, run_ctx->address.c_str()) < 0) { - run_ctx->error = zmq_errno(); + run_ctx->error = static_cast(zmq_errno()); return; } if (zmq_proxy_steerable(front_ptr, back_ptr, nullptr, control_sub) < 0) { - run_ctx->error = zmq_errno(); + run_ctx->error = static_cast(zmq_errno()); return; } }, - [=]() { - AsyncScope scope(Env(), async_context); + [this, front, back, run_ctx, res]() { + AsyncScope const scope(Env(), async_context); front->Close(); back->Close(); - auto err1 = zmq_close(control_pub); + [[maybe_unused]] auto err1 = zmq_close(control_pub); assert(err1 == 0); - auto err2 = zmq_close(control_sub); + [[maybe_unused]] auto err2 = zmq_close(control_sub); assert(err2 == 0); control_pub = nullptr; control_sub = nullptr; if (run_ctx->error != 0) { - res.Reject(ErrnoException(Env(), run_ctx->error).Value()); + res.Reject( + ErrnoException(Env(), static_cast(run_ctx->error)).Value()); return; } @@ -158,28 +175,34 @@ void Proxy::SendCommand(const char* command) { } void Proxy::Pause(const Napi::CallbackInfo& info) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return; + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return; + } SendCommand("PAUSE"); } void Proxy::Resume(const Napi::CallbackInfo& info) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return; + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return; + } SendCommand("RESUME"); } void Proxy::Terminate(const Napi::CallbackInfo& info) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return; + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return; + } SendCommand("TERMINATE"); } -Napi::Value Proxy::GetFrontEnd(const Napi::CallbackInfo& info) { +Napi::Value Proxy::GetFrontEnd(const Napi::CallbackInfo& /*info*/) { return front_ref.Value(); } -Napi::Value Proxy::GetBackEnd(const Napi::CallbackInfo& info) { +Napi::Value Proxy::GetBackEnd(const Napi::CallbackInfo& /*info*/) { return back_ref.Value(); } @@ -198,6 +221,6 @@ void Proxy::Initialize(Module& module, Napi::Object& exports) { module.Proxy = Napi::Persistent(constructor); exports.Set("Proxy", constructor); } -} +} // namespace zmq #endif diff --git a/src/proxy.h b/src/proxy.h index 77e9faf2..dd7a01c6 100644 --- a/src/proxy.h +++ b/src/proxy.h @@ -15,7 +15,12 @@ class Proxy : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Proxy(const Napi::CallbackInfo& info); - virtual ~Proxy(); + + Proxy(const Proxy&) = delete; + Proxy(Proxy&&) = delete; + Proxy& operator=(const Proxy&) = delete; + Proxy& operator=(Proxy&&) = delete; + ~Proxy() override; void Close() override; @@ -41,9 +46,9 @@ class Proxy : public Napi::ObjectWrap, public Closable { void* control_sub = nullptr; void* control_pub = nullptr; }; -} +} // namespace zmq -static_assert(!std::is_copy_constructible::value, "not copyable"); -static_assert(!std::is_move_constructible::value, "not movable"); +static_assert(!std::is_copy_constructible_v, "not copyable"); +static_assert(!std::is_move_constructible_v, "not movable"); #endif diff --git a/src/socket.cc b/src/socket.cc index 998b07f6..c740a14d 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -1,10 +1,8 @@ - - -#define NOMINMAX // prevent minwindef.h from defining max macro in the debug build - #include "./socket.h" +#include #include +#include #include #include @@ -23,7 +21,7 @@ namespace zmq { /* The maximum number of sync I/O operations that are allowed before the I/O methods will force the returned promise to be resolved in the next tick. */ -[[maybe_unused]] auto constexpr max_sync_operations = 1 << 9; +[[maybe_unused]] auto constexpr max_sync_operations = 1U << 9U; /* Ordinary static cast for all available numeric types. */ template @@ -37,9 +35,12 @@ template <> uint64_t NumberCast(const Napi::Number& num) { auto value = num.DoubleValue(); - if (std::nextafter(value, -0.0) < 0) return 0; + if (std::nextafter(value, -0.0) < 0) { + return 0; + } - if (value > static_cast((1ull << 53) - 1)) { + static constexpr auto max_safe_integer = static_cast((1ULL << 53U) - 1); + if (value > max_safe_integer) { Warn(num.Env(), "Value is larger than Number.MAX_SAFE_INTEGER and may have been rounded " "inaccurately."); @@ -47,8 +48,10 @@ uint64_t NumberCast(const Napi::Number& num) { /* If the next representable value of the double is beyond the maximum integer, then assume the maximum integer. */ + static constexpr auto max_uint64_as_double = + static_cast(std::numeric_limits::max()); if (std::nextafter(value, std::numeric_limits::infinity()) - > std::numeric_limits::max()) { + > max_uint64_as_double) { return std::numeric_limits::max(); } @@ -65,14 +68,16 @@ struct AddressContext { Socket::Socket(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info), async_context(Env(), "Socket"), poller(*this), module(*reinterpret_cast(info.Data())) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Socket type must be a number"), Arg::Optional("Options must be an object"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - type = info[0].As().Uint32Value(); + type = info[0].As().Int32Value(); if (info[1].IsObject()) { auto options = info[1].As(); @@ -86,8 +91,10 @@ Socket::Socket(const Napi::CallbackInfo& info) context_ref.Reset(module.GlobalContext.Value(), 1); } - auto context = Context::Unwrap(context_ref.Value()); - if (Env().IsExceptionPending()) return; + auto* context = Context::Unwrap(context_ref.Value()); + if (Env().IsExceptionPending()) { + return; + } socket = zmq_socket(context->context, type); if (socket == nullptr) { @@ -95,8 +102,15 @@ Socket::Socket(const Napi::CallbackInfo& info) return; } - uv_os_sock_t fd; - std::function finalize = nullptr; + uv_os_sock_t file_descriptor = 0; + std::function const finalize = nullptr; + + const auto error = [this]() { + [[maybe_unused]] auto err = zmq_close(socket); + assert(err == 0); + + socket = nullptr; + }; #ifdef ZMQ_THREAD_SAFE { @@ -104,10 +118,10 @@ Socket::Socket(const Napi::CallbackInfo& info) size_t length = sizeof(value); if (zmq_getsockopt(socket, ZMQ_THREAD_SAFE, &value, &length) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); - goto error; + error(); } - thread_safe = value; + thread_safe = (value != 0); } #endif @@ -119,44 +133,44 @@ Socket::Socket(const Napi::CallbackInfo& info) auto poll = zmq_poller_new(); if (poll == nullptr) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); - goto error; + error(); } /* Callback to free the underlying poller. Move the poller to transfer ownership after the constructor has completed. */ finalize = [=]() mutable { - auto err = zmq_poller_destroy(&poll); + [[maybe_unused]] auto err = zmq_poller_destroy(&poll); assert(err == 0); }; if (zmq_poller_add(poll, socket, nullptr, ZMQ_POLLIN | ZMQ_POLLOUT) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); finalize(); - goto error; + error(); } if (zmq_poller_fd(poll, &fd) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); finalize(); - goto error; + error(); } #else /* A thread safe socket was requested, but there is no support for retrieving a poller FD, so we cannot construct them. */ ErrnoException(Env(), EINVAL).ThrowAsJavaScriptException(); - goto error; + error(); #endif } else { - size_t length = sizeof(fd); - if (zmq_getsockopt(socket, ZMQ_FD, &fd, &length) < 0) { + size_t length = sizeof(file_descriptor); + if (zmq_getsockopt(socket, ZMQ_FD, &file_descriptor, &length) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); - goto error; + error(); } } - if (poller.Initialize(Env(), fd, finalize) < 0) { + if (poller.Initialize(Env(), file_descriptor, finalize) < 0) { ErrnoException(Env(), errno).ThrowAsJavaScriptException(); - goto error; + error(); } /* Initialization was successful, register the socket for cleanup. */ @@ -170,15 +184,6 @@ Socket::Socket(const Napi::CallbackInfo& info) if (info[1].IsObject()) { Assign(info.This().As(), info[1].As()); } - - return; - -error: - auto err = zmq_close(socket); - assert(err == 0); - - socket = nullptr; - return; } Socket::~Socket() { @@ -217,8 +222,12 @@ void Socket::WarnUnlessImmediateOption(int32_t option) const { ZMQ_RCVTIMEO, }; - if (immediate.count(option) != 0) return; - if (endpoints == 0 && state == State::Open) return; + if (immediate.contains(option)) { + return; + } + if (endpoints == 0 && state == State::Open) { + return; + } Warn(Env(), "Socket option will not take effect until next connect/bind."); } @@ -236,23 +245,25 @@ bool Socket::ValidateOpen() const { } } -bool Socket::HasEvents(int32_t requested) const { - int32_t events; +bool Socket::HasEvents(uint32_t requested_events) const { + uint32_t events = 0; size_t events_size = sizeof(events); while (zmq_getsockopt(socket, ZMQ_EVENTS, &events, &events_size) < 0) { /* Ignore errors. */ - if (zmq_errno() != EINTR) return 0; + if (zmq_errno() != EINTR) { + return false; + } } - return events & requested; + return (events & requested_events) != 0; } void Socket::Close() { if (socket != nullptr) { module.ObjectReaper.Remove(this); - Napi::HandleScope scope(Env()); + Napi::HandleScope const scope(Env()); /* Clear endpoint count. */ endpoints = 0; @@ -261,7 +272,7 @@ void Socket::Close() { poller.Close(); /* Close succeeds unless socket is invalid. */ - auto err = zmq_close(socket); + [[maybe_unused]] auto err = zmq_close(socket); assert(err == 0); /* Release reference to context and observer. */ @@ -283,8 +294,8 @@ void Socket::Send(const Napi::Promise::Deferred& res, OutgoingMsg::Parts& parts) auto& part = *iter; iter++; - uint32_t flags = iter == end ? ZMQ_DONTWAIT : ZMQ_DONTWAIT | ZMQ_SNDMORE; - while (zmq_msg_send(part, socket, flags) < 0) { + auto const flags = iter == end ? ZMQ_DONTWAIT : ZMQ_DONTWAIT | ZMQ_SNDMORE; + while (zmq_msg_send(part.get(), socket, flags) < 0) { if (zmq_errno() != EINTR) { res.Reject(ErrnoException(Env(), zmq_errno()).Value()); return; @@ -300,17 +311,17 @@ void Socket::Receive(const Napi::Promise::Deferred& res) { followed by a metadata object. */ auto list = Napi::Array::New(Env(), 1); - uint32_t i = 0; + uint32_t i_part = 0; while (true) { IncomingMsg part; - while (zmq_msg_recv(part, socket, ZMQ_DONTWAIT) < 0) { + while (zmq_msg_recv(part.get(), socket, ZMQ_DONTWAIT) < 0) { if (zmq_errno() != EINTR) { res.Reject(ErrnoException(Env(), zmq_errno()).Value()); return; } } - list[i++] = part.IntoBuffer(Env()); + list[i_part++] = part.IntoBuffer(Env()); #ifdef ZMQ_HAS_POLLABLE_THREAD_SAFE switch (type) { @@ -332,20 +343,26 @@ void Socket::Receive(const Napi::Promise::Deferred& res) { } #endif - if (!zmq_msg_more(part)) break; + if (zmq_msg_more(part.get()) == 0) { + break; + } } res.Resolve(list); } Napi::Value Socket::Bind(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Address must be a string"), }; - if (args.ThrowIfInvalid(info)) return Env().Undefined(); + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - if (!ValidateOpen()) return Env().Undefined(); + if (!ValidateOpen()) { + return Env().Undefined(); + } state = Socket::State::Blocked; auto res = Napi::Promise::Deferred::New(Env()); @@ -353,17 +370,17 @@ Napi::Value Socket::Bind(const Napi::CallbackInfo& info) { auto status = UvQueue( Env(), - [=]() { + [this, run_ctx]() { /* Don't access V8 internals here! Executed in worker thread. */ while (zmq_bind(socket, run_ctx->address.c_str()) < 0) { if (zmq_errno() != EINTR) { - run_ctx->error = zmq_errno(); + run_ctx->error = static_cast(zmq_errno()); return; } } }, - [=]() { - AsyncScope scope(Env(), async_context); + [this, run_ctx, res]() { + AsyncScope const scope(Env(), async_context); state = Socket::State::Open; endpoints++; @@ -373,8 +390,9 @@ Napi::Value Socket::Bind(const Napi::CallbackInfo& info) { } if (run_ctx->error != 0) { - res.Reject( - ErrnoException(Env(), run_ctx->error, run_ctx->address).Value()); + res.Reject(ErrnoException( + Env(), static_cast(run_ctx->error), run_ctx->address) + .Value()); return; } @@ -390,13 +408,17 @@ Napi::Value Socket::Bind(const Napi::CallbackInfo& info) { } Napi::Value Socket::Unbind(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Address must be a string"), }; - if (args.ThrowIfInvalid(info)) return Env().Undefined(); + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - if (!ValidateOpen()) return Env().Undefined(); + if (!ValidateOpen()) { + return Env().Undefined(); + } state = Socket::State::Blocked; auto res = Napi::Promise::Deferred::New(Env()); @@ -404,17 +426,17 @@ Napi::Value Socket::Unbind(const Napi::CallbackInfo& info) { auto status = UvQueue( Env(), - [=]() { + [this, run_ctx]() { /* Don't access V8 internals here! Executed in worker thread. */ while (zmq_unbind(socket, run_ctx->address.c_str()) < 0) { if (zmq_errno() != EINTR) { - run_ctx->error = zmq_errno(); + run_ctx->error = static_cast(zmq_errno()); return; } } }, - [=]() { - AsyncScope scope(Env(), async_context); + [this, run_ctx, res]() { + AsyncScope const scope(Env(), async_context); state = Socket::State::Open; --endpoints; @@ -424,8 +446,9 @@ Napi::Value Socket::Unbind(const Napi::CallbackInfo& info) { } if (run_ctx->error != 0) { - res.Reject( - ErrnoException(Env(), run_ctx->error, run_ctx->address).Value()); + res.Reject(ErrnoException( + Env(), static_cast(run_ctx->error), run_ctx->address) + .Value()); return; } @@ -441,15 +464,19 @@ Napi::Value Socket::Unbind(const Napi::CallbackInfo& info) { } void Socket::Connect(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Address must be a string"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - if (!ValidateOpen()) return; + if (!ValidateOpen()) { + return; + } - std::string address = info[0].As(); + std::string const address = info[0].As(); if (zmq_connect(socket, address.c_str()) < 0) { ErrnoException(Env(), zmq_errno(), address).ThrowAsJavaScriptException(); return; @@ -459,15 +486,19 @@ void Socket::Connect(const Napi::CallbackInfo& info) { } void Socket::Disconnect(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Address must be a string"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - if (!ValidateOpen()) return; + if (!ValidateOpen()) { + return; + } - std::string address = info[0].As(); + std::string const address = info[0].As(); if (zmq_disconnect(socket, address.c_str()) < 0) { ErrnoException(Env(), zmq_errno(), address).ThrowAsJavaScriptException(); return; @@ -477,7 +508,9 @@ void Socket::Disconnect(const Napi::CallbackInfo& info) { } void Socket::Close(const Napi::CallbackInfo& info) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return; + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return; + } /* We can't access the socket when it is blocked, delay closing. */ if (state == State::Blocked) { @@ -505,15 +538,19 @@ Napi::Value Socket::Send(const Napi::CallbackInfo& info) { #endif default: { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Message must be present"), }; - if (args.ThrowIfInvalid(info)) return Env().Undefined(); + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } } } - if (!ValidateOpen()) return Env().Undefined(); + if (!ValidateOpen()) { + return Env().Undefined(); + } if (poller.Writing()) { ErrnoException(Env(), EBUSY, @@ -568,7 +605,9 @@ Napi::Value Socket::Send(const Napi::CallbackInfo& info) { avoid starving the event loop. Writes will be delayed. */ UvScheduleDelayed(Env(), [&]() { poller.WritableCallback(); - if (socket == nullptr) return; + if (socket == nullptr) { + return; + } poller.TriggerReadable(); }); } else { @@ -579,9 +618,13 @@ Napi::Value Socket::Send(const Napi::CallbackInfo& info) { } Napi::Value Socket::Receive(const Napi::CallbackInfo& info) { - if (Arg::Validator{}.ThrowIfInvalid(info)) return Env().Undefined(); + if (Arg::Validator{}.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - if (!ValidateOpen()) return Env().Undefined(); + if (!ValidateOpen()) { + return Env().Undefined(); + } if (poller.Reading()) { ErrnoException(Env(), EBUSY, @@ -612,7 +655,9 @@ Napi::Value Socket::Receive(const Napi::CallbackInfo& info) { avoid starving the event loop. Reads will be delayed. */ UvScheduleDelayed(Env(), [&]() { poller.ReadableCallback(); - if (socket == nullptr) return; + if (socket == nullptr) { + return; + } poller.TriggerWritable(); }); } else { @@ -622,7 +667,7 @@ Napi::Value Socket::Receive(const Napi::CallbackInfo& info) { return poller.ReadPromise(); } -void Socket::Join(const Napi::CallbackInfo& info) { +void Socket::Join([[maybe_unused]] const Napi::CallbackInfo& info) { #ifdef ZMQ_HAS_POLLABLE_THREAD_SAFE Arg::Validator args{ Arg::Required("Group must be a string or buffer"), @@ -650,7 +695,7 @@ void Socket::Join(const Napi::CallbackInfo& info) { #endif } -void Socket::Leave(const Napi::CallbackInfo& info) { +void Socket::Leave([[maybe_unused]] const Napi::CallbackInfo& info) { #ifdef ZMQ_HAS_POLLABLE_THREAD_SAFE Arg::Validator args{ Arg::Required("Group must be a string or buffer"), @@ -680,13 +725,15 @@ void Socket::Leave(const Napi::CallbackInfo& info) { template <> Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), }; - if (args.ThrowIfInvalid(info)) return Env().Undefined(); + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - uint32_t option = info[0].As(); + auto const option = info[0].As(); int32_t value = 0; size_t length = sizeof(value); @@ -695,22 +742,24 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { return Env().Undefined(); } - return Napi::Boolean::New(Env(), value); + return Napi::Boolean::New(Env(), value != 0); } template <> void Socket::SetSockOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), Arg::Required("Option value must be a boolean"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - int32_t option = info[0].As(); + int32_t const option = info[0].As(); WarnUnlessImmediateOption(option); - int32_t value = info[1].As(); + int32_t value = static_cast(info[1].As()); if (zmq_setsockopt(socket, option, &value, sizeof(value)) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); return; @@ -719,55 +768,60 @@ void Socket::SetSockOpt(const Napi::CallbackInfo& info) { template <> Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), }; - if (args.ThrowIfInvalid(info)) return Env().Undefined(); + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - uint32_t option = info[0].As(); + auto const option = info[0].As(); - char value[1024]; - size_t length = sizeof(value) - 1; - if (zmq_getsockopt(socket, option, value, &length) < 0) { + static constexpr auto max_value_length = 1024; //+ + auto value = std::array(); //+ + size_t length = value.size(); //+ + if (zmq_getsockopt(socket, option, value.data(), &length) < 0) { //+ ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); return Env().Undefined(); } if (length == 0 || (length == 1 && value[0] == 0)) { return Env().Null(); - } else { - value[length] = '\0'; - return Napi::String::New(Env(), value); } + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + value[length] = '\0'; + return Napi::String::New(Env(), value.data()); } template <> void Socket::SetSockOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), Arg::Required( "Option value must be a string or buffer"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - int32_t option = info[0].As(); + int32_t const option = info[0].As(); WarnUnlessImmediateOption(option); - int32_t err; + int32_t err = 0; if (info[1].IsBuffer()) { - Napi::Object buf = info[1].As(); + auto const buf = info[1].As(); auto length = buf.As>().Length(); - auto value = buf.As>().Data(); + auto* value = buf.As>().Data(); err = zmq_setsockopt(socket, option, value, length); } else if (info[1].IsString()) { std::string str = info[1].As(); auto length = str.length(); - auto value = str.data(); + auto* value = str.data(); err = zmq_setsockopt(socket, option, value, length); } else { - auto length = 0u; + auto length = 0U; auto value = nullptr; err = zmq_setsockopt(socket, option, value, length); } @@ -780,13 +834,15 @@ void Socket::SetSockOpt(const Napi::CallbackInfo& info) { template Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), }; - if (args.ThrowIfInvalid(info)) return Env().Undefined(); + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - uint32_t option = info[0].As(); + auto const option = info[0].As(); T value = 0; size_t length = sizeof(value); @@ -800,14 +856,16 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { template void Socket::SetSockOpt(const Napi::CallbackInfo& info) { - Arg::Validator args{ + Arg::Validator const args{ Arg::Required("Identifier must be a number"), Arg::Required("Option value must be a number"), }; - if (args.ThrowIfInvalid(info)) return; + if (args.ThrowIfInvalid(info)) { + return; + } - int32_t option = info[0].As(); + int32_t const option = info[0].As(); WarnUnlessImmediateOption(option); T value = NumberCast(info[1].As()); @@ -819,15 +877,17 @@ void Socket::SetSockOpt(const Napi::CallbackInfo& info) { /* Mirror a few options that are used internally. */ switch (option) { case ZMQ_SNDTIMEO: - send_timeout = value; + send_timeout = static_cast(value); break; case ZMQ_RCVTIMEO: - receive_timeout = value; + receive_timeout = static_cast(value); + break; + default: break; } } -Napi::Value Socket::GetEvents(const Napi::CallbackInfo& info) { +Napi::Value Socket::GetEvents(const Napi::CallbackInfo& /*info*/) { /* Reuse the same observer object every time it is accessed. */ if (observer_ref.IsEmpty()) { observer_ref.Reset(module.Observer.New({Value()}), 1); @@ -836,19 +896,19 @@ Napi::Value Socket::GetEvents(const Napi::CallbackInfo& info) { return observer_ref.Value(); } -Napi::Value Socket::GetContext(const Napi::CallbackInfo& info) { +Napi::Value Socket::GetContext(const Napi::CallbackInfo& /*info*/) { return context_ref.Value(); } -Napi::Value Socket::GetClosed(const Napi::CallbackInfo& info) { +Napi::Value Socket::GetClosed(const Napi::CallbackInfo& /*info*/) { return Napi::Boolean::New(Env(), state == State::Closed); } -Napi::Value Socket::GetReadable(const Napi::CallbackInfo& info) { +Napi::Value Socket::GetReadable(const Napi::CallbackInfo& /*info*/) { return Napi::Boolean::New(Env(), HasEvents(ZMQ_POLLIN)); } -Napi::Value Socket::GetWritable(const Napi::CallbackInfo& info) { +Napi::Value Socket::GetWritable(const Napi::CallbackInfo& /*info*/) { return Napi::Boolean::New(Env(), HasEvents(ZMQ_POLLOUT)); } @@ -895,33 +955,33 @@ void Socket::Initialize(Module& module, Napi::Object& exports) { void Socket::Poller::ReadableCallback() { assert(read_deferred); - socket.sync_operations = 0; + socket.get().sync_operations = 0; - AsyncScope scope(socket.Env(), socket.async_context); - socket.Receive(take(read_deferred)); + AsyncScope const scope(socket.get().Env(), socket.get().async_context); + socket.get().Receive(take(read_deferred)); } void Socket::Poller::WritableCallback() { assert(write_deferred); - socket.sync_operations = 0; + socket.get().sync_operations = 0; - AsyncScope scope(socket.Env(), socket.async_context); - socket.Send(take(write_deferred), write_value); + AsyncScope const scope(socket.get().Env(), socket.get().async_context); + socket.get().Send(take(write_deferred), write_value); write_value.Clear(); } Napi::Value Socket::Poller::ReadPromise() { assert(!read_deferred); - read_deferred = Napi::Promise::Deferred(socket.Env()); + read_deferred = Napi::Promise::Deferred(socket.get().Env()); return read_deferred->Promise(); } -Napi::Value Socket::Poller::WritePromise(OutgoingMsg::Parts&& value) { +Napi::Value Socket::Poller::WritePromise(OutgoingMsg::Parts&& parts) { assert(!write_deferred); - write_value = std::move(value); - write_deferred = Napi::Promise::Deferred(socket.Env()); + write_value = std::move(parts); + write_deferred = Napi::Promise::Deferred(socket.get().Env()); return write_deferred->Promise(); } -} +} // namespace zmq diff --git a/src/socket.h b/src/socket.h index 52d2ee89..601cb3f2 100644 --- a/src/socket.h +++ b/src/socket.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "./closable.h" @@ -15,7 +16,12 @@ class Socket : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Socket(const Napi::CallbackInfo& info); - virtual ~Socket(); + + Socket(const Socket&) = delete; + Socket(Socket&&) = delete; + Socket& operator=(const Socket&) = delete; + Socket& operator=(Socket&&) = delete; + ~Socket() override; void Close() override; @@ -55,8 +61,8 @@ class Socket : public Napi::ObjectWrap, public Closable { private: inline void WarnUnlessImmediateOption(int32_t option) const; - inline bool ValidateOpen() const; - bool HasEvents(int32_t events) const; + [[nodiscard]] inline bool ValidateOpen() const; + [[nodiscard]] bool HasEvents(uint32_t requested_events) const; /* Send/receive are usually in a hot path and will benefit slightly from being inlined. They are used in more than one location and are @@ -65,31 +71,31 @@ class Socket : public Napi::ObjectWrap, public Closable { force_inline void Receive(const Napi::Promise::Deferred& res); class Poller : public zmq::Poller { - Socket& socket; + std::reference_wrapper socket; std::optional read_deferred; std::optional write_deferred; OutgoingMsg::Parts write_value; public: - explicit Poller(Socket& socket) : socket(socket) {} + explicit Poller(std::reference_wrapper socket) : socket(socket) {} Napi::Value ReadPromise(); Napi::Value WritePromise(OutgoingMsg::Parts&& parts); - inline bool Reading() const { + [[nodiscard]] bool Reading() const { return read_deferred.has_value(); } - inline bool Writing() const { + [[nodiscard]] bool Writing() const { return write_deferred.has_value(); } - inline bool ValidateReadable() const { - return socket.HasEvents(ZMQ_POLLIN); + [[nodiscard]] bool ValidateReadable() const { + return socket.get().HasEvents(ZMQ_POLLIN); } - inline bool ValidateWritable() const { - return socket.HasEvents(ZMQ_POLLOUT); + [[nodiscard]] bool ValidateWritable() const { + return socket.get().HasEvents(ZMQ_POLLOUT); } void ReadableCallback(); @@ -112,12 +118,12 @@ class Socket : public Napi::ObjectWrap, public Closable { State state = State::Open; bool request_close = false; bool thread_safe = false; - uint8_t type = 0; + int type = 0; friend class Observer; friend class Proxy; }; -} +} // namespace zmq -static_assert(!std::is_copy_constructible::value, "not copyable"); -static_assert(!std::is_move_constructible::value, "not movable"); +static_assert(!std::is_copy_constructible_v, "not copyable"); +static_assert(!std::is_move_constructible_v, "not movable"); diff --git a/src/util/arguments.h b/src/util/arguments.h index faa168a4..bbda1cca 100644 --- a/src/util/arguments.h +++ b/src/util/arguments.h @@ -4,11 +4,9 @@ #include -#include "to_string.h" +namespace zmq::Arg { -namespace zmq { -namespace Arg { -typedef bool (Napi::Value::*ValueMethod)() const; +using ValueMethod = bool (Napi::Value::*)() const; template struct VerifyWithMethod { @@ -30,14 +28,17 @@ struct Not { template class Verify { - const std::string_view msg; + /* const */ std::string_view msg; public: - constexpr Verify(std::string_view msg) noexcept : msg(msg) {} + constexpr explicit Verify(std::string_view msg) noexcept : msg(msg) {} - std::optional operator()(uint32_t, const Napi::Value& value) const { + std::optional operator()( + uint32_t /*unused*/, const Napi::Value& value) const { auto valid = ((F()(value)) || ...); - if (valid) return {}; + if (valid) { + return {}; + } return Napi::TypeError::New(value.Env(), std::string(msg)); } }; @@ -60,14 +61,14 @@ using Optional = Verify; template class Validator { - static constexpr size_t N = sizeof...(F); + static constexpr size_t NumArgs = sizeof...(F); std::tuple validators; public: - constexpr Validator(F&&... validators) noexcept + constexpr explicit Validator(F&&... validators) noexcept : validators(std::forward(validators)...) {} - bool ThrowIfInvalid(const Napi::CallbackInfo& info) const { + [[nodiscard]] bool ThrowIfInvalid(const Napi::CallbackInfo& info) const { if (auto err = Validate(info)) { err->ThrowAsJavaScriptException(); return true; @@ -76,25 +77,29 @@ class Validator { return false; } - std::optional Validate(const Napi::CallbackInfo& info) const { + [[nodiscard]] std::optional Validate( + const Napi::CallbackInfo& info) const { return eval(info); } private: template - std::optional eval(const Napi::CallbackInfo& info) const { - if constexpr (I == N) { - if (info.Length() > N) { - auto msg = "Expected " + to_string(N) + " argument" + (N != 1 ? "s" : ""); + [[nodiscard]] std::optional eval(const Napi::CallbackInfo& info) const { + if constexpr (I == NumArgs) { + if (info.Length() > NumArgs) { + auto msg = "Expected " + std::to_string(NumArgs) + " argument" + + (NumArgs != 1 ? "s" : ""); return Napi::TypeError::New(info.Env(), msg); } return {}; } else { - if (auto err = std::get(validators)(I, info[I])) return err; + if (auto err = std::get(validators)(I, info[I])) { + return err; + } return eval(info); } } }; -} -} +} // namespace zmq::Arg + // namespace zmq diff --git a/src/util/async_scope.h b/src/util/async_scope.h index 746edf31..86d2e3be 100644 --- a/src/util/async_scope.h +++ b/src/util/async_scope.h @@ -8,7 +8,7 @@ class AsyncScope { Napi::CallbackScope callback_scope; public: - inline explicit AsyncScope(Napi::Env env, const Napi::AsyncContext& context) + explicit AsyncScope(Napi::Env env, const Napi::AsyncContext& context) : handle_scope(env), callback_scope(env, context) {} }; } diff --git a/src/util/electron_helper.h b/src/util/electron_helper.h index 769efe22..c9a41c4e 100644 --- a/src/util/electron_helper.h +++ b/src/util/electron_helper.h @@ -5,16 +5,16 @@ #include namespace zmq { -bool hasRun = false; -bool hasElectronMemoryCageCache = false; +static bool hasRun = false; +static bool hasElectronMemoryCageCache = false; -static inline std::string first_component(std::string const& value) { - std::string::size_type pos = value.find('.'); - return pos == value.npos ? value : value.substr(0, pos); +inline std::string first_component(std::string const& value) { + std::string::size_type const pos = value.find('.'); + return pos == std::string::npos ? value : value.substr(0, pos); } /* Check if runtime is Electron. */ -static inline bool IsElectron(const Napi::Env& env) { +inline bool IsElectron(const Napi::Env& env) { auto global = env.Global(); auto isElectron = global.Get("process") .As() @@ -24,7 +24,7 @@ static inline bool IsElectron(const Napi::Env& env) { return isElectron; } -static inline bool hasElectronMemoryCage(const Napi::Env& env) { +inline bool hasElectronMemoryCage(const Napi::Env& env) { if (!hasRun) { if (IsElectron(env)) { auto electronVers = env.Global() @@ -35,8 +35,9 @@ static inline bool hasElectronMemoryCage(const Napi::Env& env) { .Get("electron") .ToString() .Utf8Value(); - int majorVer = stoi(first_component(electronVers)); - if (majorVer >= 21) { + int const majorVer = stoi(first_component(electronVers)); + static constexpr auto electron_memory_cage_version = 21; + if (majorVer >= electron_memory_cage_version) { hasElectronMemoryCageCache = true; } } @@ -44,4 +45,4 @@ static inline bool hasElectronMemoryCage(const Napi::Env& env) { } return hasElectronMemoryCageCache; } -} +} // namespace zmq diff --git a/src/util/error.h b/src/util/error.h index 02197476..5930ef3f 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -1,51 +1,52 @@ #pragma once -#include #include +#include #include #include "../zmq_inc.h" namespace zmq { -static inline constexpr const char* ErrnoMessage(int32_t errorno); -static inline constexpr const char* ErrnoCode(int32_t errorno); +constexpr const char* ErrnoMessage(int32_t errorno); +constexpr const char* ErrnoCode(int32_t errorno); /* Generates a process warning message. */ -static inline void Warn(const Napi::Env& env, const std::string& msg) { +inline void Warn(const Napi::Env& env, const std::string& msg) { auto global = env.Global(); - auto fn = + auto emitWarning = global.Get("process").As().Get("emitWarning").As(); - fn.Call({Napi::String::New(env, msg)}); + emitWarning.Call({Napi::String::New(env, msg)}); } -static inline Napi::Error StatusException( +inline Napi::Error StatusException( const Napi::Env& env, const std::string& msg, uint32_t status) { - Napi::HandleScope scope(env); + Napi::HandleScope const scope(env); auto exception = Napi::Error::New(env, msg); exception.Set("status", Napi::Number::New(env, status)); return exception; } -static inline Napi::Error CodedException( +inline Napi::Error CodedException( const Napi::Env& env, const std::string& msg, const std::string& code) { - Napi::HandleScope scope(env); + Napi::HandleScope const scope(env); auto exception = Napi::Error::New(env, msg); exception.Set("code", Napi::String::New(env, code)); return exception; } /* This mostly duplicates node::ErrnoException, but it is not public. */ -static inline Napi::Error ErrnoException( +inline Napi::Error ErrnoException( const Napi::Env& env, int32_t error, const char* message = nullptr) { - Napi::HandleScope scope(env); - auto exception = Napi::Error::New(env, message ? message : ErrnoMessage(error)); + Napi::HandleScope const scope(env); + auto exception = + Napi::Error::New(env, (message != nullptr) ? message : ErrnoMessage(error)); exception.Set("errno", Napi::Number::New(env, error)); exception.Set("code", Napi::String::New(env, ErrnoCode(error))); return exception; } -static inline Napi::Error ErrnoException( +inline Napi::Error ErrnoException( const Napi::Env& env, int32_t error, const std::string& address) { auto exception = ErrnoException(env, error, nullptr); exception.Set("address", Napi::String::New(env, address)); @@ -53,7 +54,7 @@ static inline Napi::Error ErrnoException( } /* Convert errno to human readable error message. */ -static inline constexpr const char* ErrnoMessage(int32_t errorno) { +constexpr const char* ErrnoMessage(int32_t errorno) { /* Clarify a few confusing default messages; otherwise rely on zmq. */ switch (errorno) { case EFAULT: @@ -78,350 +79,428 @@ static inline constexpr const char* ErrnoMessage(int32_t errorno) { /* This is copied from Node.js; the mapping is not in a public API. */ /* Copyright Node.js contributors. All rights reserved. */ -static inline constexpr const char* ErrnoCode(int32_t errorno) { -#define ERRNO_CASE(e) \ - case e: \ - return #e; - +constexpr const char* ErrnoCode(int32_t errorno) { switch (errorno) { /* ZMQ specific codes. */ #ifdef EFSM - ERRNO_CASE(EFSM); + case EFSM: + return "EFSM"; #endif #ifdef ENOCOMPATPROTO - ERRNO_CASE(ENOCOMPATPROTO); + case ENOCOMPATPROTO: + return "ENOCOMPATPROTO"; #endif #ifdef ETERM - ERRNO_CASE(ETERM); + case ETERM: + return "ETERM"; #endif #ifdef EMTHREAD - ERRNO_CASE(EMTHREAD); + case EMTHREAD: + return "EMTHREAD"; #endif /* Generic codes. */ #ifdef EACCES - ERRNO_CASE(EACCES); + case EACCES: + return "EACCES"; #endif #ifdef EADDRINUSE - ERRNO_CASE(EADDRINUSE); + case EADDRINUSE: + return "EADDRINUSE"; #endif #ifdef EADDRNOTAVAIL - ERRNO_CASE(EADDRNOTAVAIL); + case EADDRNOTAVAIL: + return "EADDRNOTAVAIL"; #endif #ifdef EAFNOSUPPORT - ERRNO_CASE(EAFNOSUPPORT); + case EAFNOSUPPORT: + return "EAFNOSUPPORT"; #endif #ifdef EAGAIN - ERRNO_CASE(EAGAIN); + case EAGAIN: + return "EAGAIN"; #endif #ifdef EWOULDBLOCK #if EAGAIN != EWOULDBLOCK - ERRNO_CASE(EWOULDBLOCK); + case EWOULDBLOCK: + return "EWOULDBLOCK"; #endif #endif #ifdef EALREADY - ERRNO_CASE(EALREADY); + case EALREADY: + return "EALREADY"; #endif #ifdef EBADF - ERRNO_CASE(EBADF); + case EBADF: + return "EBADF"; #endif #ifdef EBADMSG - ERRNO_CASE(EBADMSG); + case EBADMSG: + return "EBADMSG"; #endif #ifdef EBUSY - ERRNO_CASE(EBUSY); + case EBUSY: + return "EBUSY"; #endif #ifdef ECANCELED - ERRNO_CASE(ECANCELED); + case ECANCELED: + return "ECANCELED"; #endif #ifdef ECHILD - ERRNO_CASE(ECHILD); + case ECHILD: + return "ECHILD"; #endif #ifdef ECONNABORTED - ERRNO_CASE(ECONNABORTED); + case ECONNABORTED: + return "ECONNABORTED"; #endif #ifdef ECONNREFUSED - ERRNO_CASE(ECONNREFUSED); + case ECONNREFUSED: + return "ECONNREFUSED"; #endif #ifdef ECONNRESET - ERRNO_CASE(ECONNRESET); + case ECONNRESET: + return "ECONNRESET"; #endif #ifdef EDEADLK - ERRNO_CASE(EDEADLK); + case EDEADLK: + return "EDEADLK"; #endif #ifdef EDESTADDRREQ - ERRNO_CASE(EDESTADDRREQ); + case EDESTADDRREQ: + return "EDESTADDRREQ"; #endif #ifdef EDOM - ERRNO_CASE(EDOM); + case EDOM: + return "EDOM"; #endif #ifdef EDQUOT - ERRNO_CASE(EDQUOT); + case EDQUOT: + return "EDQUOT"; #endif #ifdef EEXIST - ERRNO_CASE(EEXIST); + case EEXIST: + return "EEXIST"; #endif #ifdef EFAULT - ERRNO_CASE(EFAULT); + case EFAULT: + return "EFAULT"; #endif #ifdef EFBIG - ERRNO_CASE(EFBIG); + case EFBIG: + return "EFBIG"; #endif #ifdef EHOSTUNREACH - ERRNO_CASE(EHOSTUNREACH); + case EHOSTUNREACH: + return "EHOSTUNREACH"; #endif #ifdef EIDRM - ERRNO_CASE(EIDRM); + case EIDRM: + return "EIDRM"; #endif #ifdef EILSEQ - ERRNO_CASE(EILSEQ); + case EILSEQ: + return "EILSEQ"; #endif #ifdef EINPROGRESS - ERRNO_CASE(EINPROGRESS); + case EINPROGRESS: + return "EINPROGRESS"; #endif #ifdef EINTR - ERRNO_CASE(EINTR); + case EINTR: + return "EINTR"; #endif #ifdef EINVAL - ERRNO_CASE(EINVAL); + case EINVAL: + return "EINVAL"; #endif #ifdef EIO - ERRNO_CASE(EIO); + case EIO: + return "EIO"; #endif #ifdef EISCONN - ERRNO_CASE(EISCONN); + case EISCONN: + return "EISCONN"; #endif #ifdef EISDIR - ERRNO_CASE(EISDIR); + case EISDIR: + return "EISDIR"; #endif #ifdef ELOOP - ERRNO_CASE(ELOOP); + case ELOOP: + return "ELOOP"; #endif #ifdef EMFILE - ERRNO_CASE(EMFILE); + case EMFILE: + return "EMFILE"; #endif #ifdef EMLINK - ERRNO_CASE(EMLINK); + case EMLINK: + return "EMLINK"; #endif #ifdef EMSGSIZE - ERRNO_CASE(EMSGSIZE); + case EMSGSIZE: + return "EMSGSIZE"; #endif #ifdef EMULTIHOP - ERRNO_CASE(EMULTIHOP); + case EMULTIHOP: + return "EMULTIHOP"; #endif #ifdef ENAMETOOLONG - ERRNO_CASE(ENAMETOOLONG); + case ENAMETOOLONG: + return "ENAMETOOLONG"; #endif #ifdef ENETDOWN - ERRNO_CASE(ENETDOWN); + case ENETDOWN: + return "ENETDOWN"; #endif #ifdef ENETRESET - ERRNO_CASE(ENETRESET); + case ENETRESET: + return "ENETRESET"; #endif #ifdef ENETUNREACH - ERRNO_CASE(ENETUNREACH); + case ENETUNREACH: + return "ENETUNREACH"; #endif #ifdef ENFILE - ERRNO_CASE(ENFILE); + case ENFILE: + return "ENFILE"; #endif #ifdef ENOBUFS - ERRNO_CASE(ENOBUFS); + case ENOBUFS: + return "ENOBUFS"; #endif #ifdef ENODATA - ERRNO_CASE(ENODATA); + case ENODATA: + return "ENODATA"; #endif #ifdef ENODEV - ERRNO_CASE(ENODEV); + case ENODEV: + return "ENODEV"; #endif #ifdef ENOENT - ERRNO_CASE(ENOENT); + case ENOENT: + return "ENOENT"; #endif #ifdef ENOEXEC - ERRNO_CASE(ENOEXEC); + case ENOEXEC: + return "ENOEXEC"; #endif #ifdef ENOLINK - ERRNO_CASE(ENOLINK); + case ENOLINK: + return "ENOLINK"; #endif #ifdef ENOLCK #if ENOLINK != ENOLCK - ERRNO_CASE(ENOLCK); + case ENOLCK: + return "ENOLCK"; #endif #endif #ifdef ENOMEM - ERRNO_CASE(ENOMEM); + case ENOMEM: + return "ENOMEM"; #endif #ifdef ENOMSG - ERRNO_CASE(ENOMSG); + case ENOMSG: + return "ENOMSG"; #endif #ifdef ENOPROTOOPT - ERRNO_CASE(ENOPROTOOPT); + case ENOPROTOOPT: + return "ENOPROTOOPT"; #endif #ifdef ENOSPC - ERRNO_CASE(ENOSPC); + case ENOSPC: + return "ENOSPC"; #endif #ifdef ENOSR - ERRNO_CASE(ENOSR); + case ENOSR: + return "ENOSR"; #endif #ifdef ENOSTR - ERRNO_CASE(ENOSTR); + case ENOSTR: + return "ENOSTR"; #endif #ifdef ENOSYS - ERRNO_CASE(ENOSYS); + case ENOSYS: + return "ENOSYS"; #endif #ifdef ENOTCONN - ERRNO_CASE(ENOTCONN); + case ENOTCONN: + return "ENOTCONN"; #endif #ifdef ENOTDIR - ERRNO_CASE(ENOTDIR); + case ENOTDIR: + return "ENOTDIR"; #endif #ifdef ENOTEMPTY #if ENOTEMPTY != EEXIST - ERRNO_CASE(ENOTEMPTY); + case ENOTEMPTY: + return "ENOTEMPTY"; #endif #endif #ifdef ENOTSOCK - ERRNO_CASE(ENOTSOCK); + case ENOTSOCK: + return "ENOTSOCK"; #endif #ifdef ENOTSUP - ERRNO_CASE(ENOTSUP); + case ENOTSUP: + return "ENOTSUP"; #else #ifdef EOPNOTSUPP - ERRNO_CASE(EOPNOTSUPP); + case EOPNOTSUPP: + return "EOPNOTSUPP"; #endif #endif #ifdef ENOTTY - ERRNO_CASE(ENOTTY); + case ENOTTY: + return "ENOTTY"; #endif #ifdef ENXIO - ERRNO_CASE(ENXIO); + case ENXIO: + return "ENXIO"; #endif #ifdef EOVERFLOW - ERRNO_CASE(EOVERFLOW); + case EOVERFLOW: + return "EOVERFLOW"; #endif #ifdef EPERM - ERRNO_CASE(EPERM); + case EPERM: + return "EPERM"; #endif #ifdef EPIPE - ERRNO_CASE(EPIPE); + case EPIPE: + return "EPIPE"; #endif #ifdef EPROTO - ERRNO_CASE(EPROTO); + case EPROTO: + return "EPROTO"; #endif #ifdef EPROTONOSUPPORT - ERRNO_CASE(EPROTONOSUPPORT); + case EPROTONOSUPPORT: + return "EPROTONOSUPPORT"; #endif #ifdef EPROTOTYPE - ERRNO_CASE(EPROTOTYPE); + case EPROTOTYPE: + return "EPROTOTYPE"; #endif #ifdef ERANGE - ERRNO_CASE(ERANGE); + case ERANGE: + return "ERANGE"; #endif #ifdef EROFS - ERRNO_CASE(EROFS); + case EROFS: + return "EROFS"; #endif #ifdef ESPIPE - ERRNO_CASE(ESPIPE); + case ESPIPE: + return "ESPIPE"; #endif #ifdef ESRCH - ERRNO_CASE(ESRCH); + case ESRCH: + return "ESRCH"; #endif #ifdef ESTALE - ERRNO_CASE(ESTALE); + case ESTALE: + return "ESTALE"; #endif #ifdef ETIME - ERRNO_CASE(ETIME); + case ETIME: + return "ETIME"; #endif #ifdef ETIMEDOUT - ERRNO_CASE(ETIMEDOUT); + case ETIMEDOUT: + return "ETIMEDOUT"; #endif #ifdef ETXTBSY - ERRNO_CASE(ETXTBSY); + case ETXTBSY: + return "ETXTBSY"; #endif #ifdef EXDEV - ERRNO_CASE(EXDEV); + case EXDEV: + return "EXDEV"; #endif default: return ""; } } -} +} // namespace zmq diff --git a/src/util/object.h b/src/util/object.h index 5093bb5c..70c26379 100644 --- a/src/util/object.h +++ b/src/util/object.h @@ -4,17 +4,17 @@ namespace zmq { /* Seals an object to prevent setting incorrect options. */ -static inline void Seal(Napi::Object object) { +inline void Seal(Napi::Object object) { auto global = object.Env().Global(); auto seal = global.Get("Object").As().Get("seal").As(); seal.Call({object}); } /* Assign all properties in the given options object. */ -static inline void Assign(Napi::Object object, Napi::Object options) { +inline void Assign(Napi::Object object, Napi::Object options) { auto global = object.Env().Global(); auto assign = global.Get("Object").As().Get("assign").As(); assign.Call({object, options}); } -} +} // namespace zmq diff --git a/src/util/reaper.h b/src/util/reaper.h index 2f145c19..7693f5a1 100644 --- a/src/util/reaper.h +++ b/src/util/reaper.h @@ -21,6 +21,12 @@ class Reaper { std::set pointers; public: + Reaper() = default; + Reaper(const Reaper&) = delete; + Reaper(Reaper&&) = delete; + Reaper& operator=(const Reaper&) = delete; + Reaper& operator=(Reaper&&) = delete; + ~Reaper() { /* Copy pointers to vector to avoid issues with callbacks deregistering themselves from the reaper while we are still iterating. We iterate @@ -32,12 +38,12 @@ class Reaper { } } - inline void Add(T* ptr) { + void Add(T* ptr) { assert(ptr); pointers.insert(ptr); } - inline void Remove(T* ptr) { + void Remove(T* ptr) { assert(ptr); pointers.erase(ptr); } @@ -49,14 +55,14 @@ class ThreadSafeReaper : Reaper { std::mutex lock; public: - inline void Add(T* ptr) { + void Add(T* ptr) { std::lock_guard guard(lock); Reaper::Add(ptr); } - inline void Remove(T* ptr) { + void Remove(T* ptr) { std::lock_guard guard(lock); Reaper::Remove(ptr); } }; -} +} // namespace zmq diff --git a/src/util/to_string.h b/src/util/to_string.h deleted file mode 100644 index e1cba4ff..00000000 --- a/src/util/to_string.h +++ /dev/null @@ -1,19 +0,0 @@ -#pragma once - -#include - -namespace zmq { -/* Provide an alternative, simplified std::to_string implementation for - integers to work around https://bugs.alpinelinux.org/issues/8626. */ -static inline std::string to_string(int64_t val) { - if (val == 0) return "0"; - std::string str; - - while (val > 0) { - str.insert(0, 1, val % 10 + 48); - val /= 10; - } - - return str; -} -} diff --git a/src/util/trash.h b/src/util/trash.h index 618bf334..eff3e80c 100644 --- a/src/util/trash.h +++ b/src/util/trash.h @@ -20,8 +20,8 @@ class Trash { public: /* Construct trash with an associated asynchronous callback. */ - inline Trash(const Napi::Env& env) { - auto loop = UvLoop(env); + explicit Trash(const Napi::Env& env) { + auto* loop = UvLoop(env); async->data = this; @@ -29,17 +29,17 @@ class Trash { reinterpret_cast(async->data)->Clear(); }; - auto err = uv_async_init(loop, async, clear); + [[maybe_unused]] auto err = uv_async_init(loop, async.get(), clear); assert(err == 0); /* Immediately unreference this handle in order to prevent the async callback from preventing the Node.js process to exit. */ - uv_unref(async); + uv_unref(this->async.get_handle()); } /* Add given item to the trash, marking it for deletion next time the async callback is called by UV. */ - inline void Add(T* item) { + void Add(T* item) { std::lock_guard guard(lock); values.emplace_back(item); @@ -47,14 +47,14 @@ class Trash { that calls are coalesced if they occur frequently. This is good news for us, since that means frequent additions do not cause unnecessary trash cycle operations. */ - auto err = uv_async_send(async); + [[maybe_unused]] auto err = uv_async_send(this->async.get()); assert(err == 0); } /* Empty the trash. */ - inline void Clear() { + void Clear() { std::lock_guard guard(lock); values.clear(); } }; -} +} // namespace zmq diff --git a/src/util/uvdelayed.h b/src/util/uvdelayed.h index e5a74241..f30ff376 100644 --- a/src/util/uvdelayed.h +++ b/src/util/uvdelayed.h @@ -13,26 +13,26 @@ class UvDelayed { public: UvDelayed(const Napi::Env& env, C&& callback) : delayed_callback(std::move(callback)) { - auto loop = UvLoop(env); - int32_t err; + auto* loop = UvLoop(env); + [[maybe_unused]] int32_t err = 0; check->data = this; - err = uv_check_init(loop, check); + err = uv_check_init(loop, check.get()); assert(err == 0); idle->data = this; - err = uv_idle_init(loop, idle); + err = uv_idle_init(loop, idle.get()); assert(err == 0); } - inline void Schedule() { - int32_t err; + void Schedule() { + [[maybe_unused]] int32_t err = 0; /* Idle handle is needed to stop the event loop from blocking in poll. */ - err = uv_idle_start(idle, [](uv_idle_t* idle) {}); + err = uv_idle_start(idle.get(), []([[maybe_unused]] uv_idle_t* idle) {}); assert(err == 0); - err = uv_check_start(check, [](uv_check_t* check) { + err = uv_check_start(check.get(), [](uv_check_t* check) { auto& immediate = *reinterpret_cast(check->data); immediate.check.reset(); immediate.idle.reset(); @@ -46,8 +46,8 @@ class UvDelayed { /* This is similar to JS setImmediate(). */ template -static inline void UvScheduleDelayed(const Napi::Env& env, C callback) { +inline void UvScheduleDelayed(const Napi::Env& env, C callback) { auto immediate = new UvDelayed(env, std::move(callback)); return immediate->Schedule(); } -} +} // namespace zmq diff --git a/src/util/uvhandle.h b/src/util/uvhandle.h index 90dfd813..c7c641b9 100644 --- a/src/util/uvhandle.h +++ b/src/util/uvhandle.h @@ -7,13 +7,13 @@ namespace zmq { template struct UvDeleter { - constexpr UvDeleter() {}; + constexpr UvDeleter() = default; - inline void operator()(T* handle) { + void operator()(T* handle) { /* If uninitialized, simply delete the memory. We may not call uv_close() on uninitialized handles. */ if (handle->type == 0) { - delete reinterpret_cast(handle); + delete handle; return; } @@ -30,21 +30,21 @@ using handle_ptr = std::unique_ptr>; template class UvHandle : handle_ptr { public: - inline UvHandle() : handle_ptr{new T{}, UvDeleter()} {}; + UvHandle() : handle_ptr{new T{}, UvDeleter()} {} using handle_ptr::reset; using handle_ptr::operator->; - inline operator bool() { + explicit operator bool() { return handle_ptr::operator bool() && handle_ptr::get()->type != 0; } - inline operator T*() { + T* get() { return handle_ptr::get(); } - inline operator uv_handle_t*() { + uv_handle_t* get_handle() { return reinterpret_cast(handle_ptr::get()); } }; -} +} // namespace zmq diff --git a/src/util/uvloop.h b/src/util/uvloop.h index 3df949f1..896809c1 100644 --- a/src/util/uvloop.h +++ b/src/util/uvloop.h @@ -8,7 +8,7 @@ namespace zmq { inline uv_loop_t* UvLoop(const Napi::Env& env) { uv_loop_t* loop = nullptr; - auto status = napi_get_uv_event_loop(env, &loop); + [[maybe_unused]] auto status = napi_get_uv_event_loop(env, &loop); assert(status == napi_ok); return loop; } diff --git a/src/util/uvwork.h b/src/util/uvwork.h index a3eadda4..801c2de3 100644 --- a/src/util/uvwork.h +++ b/src/util/uvwork.h @@ -14,34 +14,36 @@ class UvWork { C complete_callback; public: - inline UvWork(E execute, C complete) + UvWork(E execute, C complete) : execute_callback(std::move(execute)), complete_callback(std::move(complete)) { work->data = this; } - inline int32_t Schedule(uv_loop_t* loop) { + int32_t Schedule(uv_loop_t* loop) { auto err = uv_queue_work( loop, work.get(), [](uv_work_t* req) { auto& work = *reinterpret_cast(req->data); work.execute_callback(); }, - [](uv_work_t* req, int status) { + [](uv_work_t* req, int /*status*/) { auto& work = *reinterpret_cast(req->data); work.complete_callback(); delete &work; }); - if (err != 0) delete this; + if (err != 0) { + delete this; + } return err; } }; template -static inline int32_t UvQueue(const Napi::Env& env, E execute, C complete) { - auto loop = UvLoop(env); +inline int32_t UvQueue(const Napi::Env& env, E execute, C complete) { + auto* loop = UvLoop(env); auto work = new UvWork(std::move(execute), std::move(complete)); return work->Schedule(loop); } -} +} // namespace zmq