From e79e375bc1e58118693a230f0ac930e73f2a8ecb Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Mon, 21 Oct 2024 00:00:51 -0700 Subject: [PATCH 01/27] fix: fix several clang-tidy issues --- src/context.cc | 50 ++++++---- src/context.h | 8 +- src/incoming_msg.cc | 6 +- src/incoming_msg.h | 10 +- src/module.cc | 16 ++-- src/module.h | 10 +- src/observer.cc | 58 ++++++----- src/observer.h | 20 ++-- src/outgoing_msg.cc | 8 +- src/outgoing_msg.h | 19 ++-- src/poller.h | 42 ++++---- src/proxy.cc | 66 ++++++++----- src/proxy.h | 8 +- src/socket.cc | 191 +++++++++++++++++++++---------------- src/socket.h | 20 ++-- src/util/arguments.h | 32 ++++--- src/util/async_scope.h | 2 +- src/util/electron_helper.h | 8 +- src/util/error.h | 21 ++-- src/util/object.h | 2 +- src/util/reaper.h | 10 +- src/util/to_string.h | 6 +- src/util/trash.h | 14 +-- src/util/uvdelayed.h | 10 +- src/util/uvhandle.h | 13 ++- src/util/uvwork.h | 14 +-- 26 files changed, 378 insertions(+), 286 deletions(-) diff --git a/src/context.cc b/src/context.cc index ba6295ee..20099f47 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(); } @@ -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(); + uint32_t const 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(); + uint32_t const 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(); + uint32_t const 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(); + uint32_t const 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..9f637b93 100644 --- a/src/context.h +++ b/src/context.h @@ -12,7 +12,7 @@ class Context : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Context(const Napi::CallbackInfo& info); - virtual ~Context(); + ~Context() override; Context(Context&&) = delete; Context& operator=(Context&&) = delete; @@ -34,7 +34,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..be417d6e 100644 --- a/src/incoming_msg.cc +++ b/src/incoming_msg.cc @@ -26,7 +26,7 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { return env.Undefined(); } } - auto data = reinterpret_cast(zmq_msg_data(*ref)); + auto* data = reinterpret_cast(zmq_msg_data(*ref)); auto length = zmq_msg_size(*ref); if (noElectronMemoryCage) { @@ -41,7 +41,7 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { Napi::MemoryManagement::AdjustExternalMemory(env, length); auto release = [](const Napi::Env& env, uint8_t*, Reference* ref) { - ptrdiff_t length = zmq_msg_size(*ref); + ptrdiff_t const length = zmq_msg_size(*ref); Napi::MemoryManagement::AdjustExternalMemory(env, -length); delete ref; }; @@ -66,4 +66,4 @@ IncomingMsg::Reference::~Reference() { 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..f3a33903 100644 --- a/src/incoming_msg.h +++ b/src/incoming_msg.h @@ -15,18 +15,20 @@ class IncomingMsg { Napi::Value IntoBuffer(const Napi::Env& env); + // NOLINTNEXTLINE(*-explicit-*) inline operator zmq_msg_t*() { return *ref; } private: class Reference { - zmq_msg_t msg; + zmq_msg_t msg{}; public: Reference(); ~Reference(); + // NOLINTNEXTLINE(*-explicit-*) inline operator zmq_msg_t*() { return &msg; } @@ -35,7 +37,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/module.cc b/src/module.cc index 1904eb19..86372dc0 100644 --- a/src/module.cc +++ b/src/module.cc @@ -6,12 +6,15 @@ #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; + int32_t major = 0; + int32_t minor = 0; + int32_t patch = 0; zmq_version(&major, &minor, &patch); return Napi::String::New( @@ -23,7 +26,7 @@ static inline Napi::Object Capabilities(const Napi::Env& 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))); } @@ -68,8 +71,7 @@ static inline Napi::Value CurveKeyPair(const Napi::CallbackInfo& info) { return result; } -Module::Global::Global() { - SharedContext = zmq_ctx_new(); +Module::Global::Global() : SharedContext(zmq_ctx_new()) { assert(SharedContext != nullptr); ContextTerminator.Add(SharedContext); @@ -82,7 +84,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,11 +110,11 @@ 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.*/ diff --git a/src/module.h b/src/module.h index f60080e2..6f0ba707 100644 --- a/src/module.h +++ b/src/module.h @@ -18,7 +18,7 @@ struct Terminator { assert(context != nullptr); #ifdef ZMQ_BLOCKY - bool blocky = zmq_ctx_get(context, ZMQ_BLOCKY); + bool const blocky = zmq_ctx_get(context, ZMQ_BLOCKY) != 0; #else /* If the option cannot be set, don't suggest to set it. */ bool blocky = false; @@ -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..d6163c84 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -10,7 +10,7 @@ #include "util/take.h" namespace zmq { -static inline constexpr const char* EventName(uint32_t val) { +static constexpr const char* EventName(uint32_t val) { switch (val) { case ZMQ_EVENT_CONNECTED: return "connect"; @@ -74,7 +74,7 @@ static inline constexpr const char* EventName(uint32_t val) { } #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_AUTH -static inline constexpr const char* AuthError(uint32_t val) { +static constexpr const char* AuthError(uint32_t val) { switch (val) { case 300: return "Temporary error"; @@ -126,14 +126,18 @@ 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.") @@ -144,14 +148,14 @@ Observer::Observer(const Napi::CallbackInfo& info) 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; + uv_os_sock_t fd = 0; size_t length = sizeof(fd); if (zmq_connect(socket, address.c_str()) < 0) { @@ -179,7 +183,6 @@ Observer::Observer(const Napi::CallbackInfo& info) assert(err == 0); socket = nullptr; - return; } Observer::~Observer() { @@ -196,22 +199,24 @@ bool Observer::ValidateOpen() const { } bool Observer::HasEvents() const { - int32_t events; + int32_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 0; + } } - 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); @@ -240,7 +245,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 +259,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()); @@ -306,15 +311,21 @@ void Observer::Receive(const Napi::Promise::Deferred& res) { } 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 +340,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,7 +364,7 @@ void Observer::Initialize(Module& module, Napi::Object& exports) { void Observer::Poller::ReadableCallback() { assert(read_deferred); - AsyncScope scope(socket.Env(), socket.async_context); + AsyncScope const scope(socket.Env(), socket.async_context); socket.Receive(take(read_deferred)); } @@ -364,4 +374,4 @@ Napi::Value Observer::Poller::ReadPromise() { read_deferred = Napi::Promise::Deferred(socket.Env()); return read_deferred->Promise(); } -} +} // namespace zmq diff --git a/src/observer.h b/src/observer.h index e902b5e3..84ba9b5a 100644 --- a/src/observer.h +++ b/src/observer.h @@ -16,7 +16,7 @@ class Observer : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Observer(const Napi::CallbackInfo& info); - virtual ~Observer(); + ~Observer() override; void Close() override; @@ -27,8 +27,8 @@ 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); @@ -41,20 +41,20 @@ class Observer : public Napi::ObjectWrap, public Closable { Napi::Value ReadPromise(); - inline bool Reading() const { + [[nodiscard]] bool Reading() const { return read_deferred.has_value(); } - inline bool ValidateReadable() const { + [[nodiscard]] bool ValidateReadable() const { return socket.HasEvents(); } - inline bool ValidateWritable() const { + [[nodiscard]] static bool ValidateWritable() { return false; } void ReadableCallback(); - inline void WritableCallback() {} + void WritableCallback() {} }; Napi::AsyncContext async_context; @@ -65,7 +65,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..f8f269c3 100644 --- a/src/outgoing_msg.cc +++ b/src/outgoing_msg.cc @@ -18,7 +18,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 +44,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 = const_cast(str->data()); auto release = [](void*, void* str) { delete reinterpret_cast(str); @@ -100,7 +100,7 @@ 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 +157,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..4012a35d 100644 --- a/src/outgoing_msg.h +++ b/src/outgoing_msg.h @@ -24,6 +24,7 @@ class OutgoingMsg { explicit OutgoingMsg(Napi::Value value, Module& module); ~OutgoingMsg(); + // NOLINTNEXTLINE(*-explicit-*) inline operator zmq_msg_t*() { return &msg; } @@ -34,13 +35,13 @@ class OutgoingMsg { Module& module; public: - inline explicit Reference(Napi::Value val, Module& module) + explicit Reference(Napi::Value val, Module& module) : persistent(Napi::Persistent(val)), module(module) {} void Recycle(); }; - zmq_msg_t msg; + zmq_msg_t msg{}; friend class Module; }; @@ -51,14 +52,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 +68,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..956e0b1c 100644 --- a/src/poller.h +++ b/src/poller.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "util/uvhandle.h" #include "util/uvloop.h" @@ -20,9 +22,9 @@ class Poller { /* 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( + int32_t Initialize( Napi::Env env, uv_os_sock_t& fd, std::function finalizer = nullptr) { - auto loop = UvLoop(env); + auto* loop = UvLoop(env); poll->data = this; if (auto err = uv_poll_init_socket(loop, poll, fd); err != 0) { @@ -39,13 +41,13 @@ class Poller { 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,11 +60,13 @@ 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) { @@ -77,7 +81,7 @@ class Poller { 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); assert(err == 0); @@ -86,7 +90,7 @@ class Poller { events |= UV_READABLE; } - inline void PollWritable(int64_t timeout) { + void PollWritable(int64_t timeout) { assert((events & UV_WRITABLE) == 0); if (timeout > 0) { @@ -104,7 +108,7 @@ 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) { + if (events == 0) { auto err = uv_poll_start(poll, UV_READABLE, Callback); assert(err == 0); } @@ -114,16 +118,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,20 +138,20 @@ 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(int32_t triggered) { events &= ~triggered; - if (!events) { + if (events == 0) { auto err = uv_poll_stop(poll); assert(err == 0); } - if (triggered & UV_READABLE) { + if ((triggered & UV_READABLE) != 0) { auto err = uv_timer_stop(readable_timer); assert(err == 0); static_cast(this)->ReadableCallback(); } - if (triggered & UV_WRITABLE) { + if ((triggered & UV_WRITABLE) != 0) { auto err = uv_timer_stop(writable_timer); assert(err == 0); static_cast(this)->WritableCallback(); @@ -157,7 +161,7 @@ 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) { auto& poller = *reinterpret_cast(poll->data); poller.TriggerReadable(); @@ -165,4 +169,4 @@ class Poller { } } }; -} +} // namespace zmq diff --git a/src/proxy.cc b/src/proxy.cc index 3f969cc3..3df2ceb6 100644 --- a/src/proxy.cc +++ b/src/proxy.cc @@ -22,34 +22,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 +75,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) { @@ -94,12 +108,12 @@ 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(); @@ -111,8 +125,8 @@ Napi::Value Proxy::Run(const Napi::CallbackInfo& info) { return; } }, - [=]() { - AsyncScope scope(Env(), async_context); + [this, front, back, run_ctx, res]() { + AsyncScope const scope(Env(), async_context); front->Close(); back->Close(); @@ -158,28 +172,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 +218,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..9436bf97 100644 --- a/src/proxy.h +++ b/src/proxy.h @@ -15,7 +15,7 @@ class Proxy : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Proxy(const Napi::CallbackInfo& info); - virtual ~Proxy(); + ~Proxy() override; void Close() override; @@ -41,9 +41,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..89e7397d 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -37,9 +37,10 @@ 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)) { + if (value > static_cast((1ULL << 53) - 1)) { Warn(num.Env(), "Value is larger than Number.MAX_SAFE_INTEGER and may have been rounded " "inaccurately."); @@ -65,12 +66,13 @@ 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(); @@ -86,8 +88,9 @@ 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 +98,8 @@ Socket::Socket(const Napi::CallbackInfo& info) return; } - uv_os_sock_t fd; - std::function finalize = nullptr; + uv_os_sock_t fd = 0; + std::function const finalize = nullptr; #ifdef ZMQ_THREAD_SAFE { @@ -107,7 +110,7 @@ Socket::Socket(const Napi::CallbackInfo& info) goto error; } - thread_safe = value; + thread_safe = (value != 0); } #endif @@ -178,7 +181,6 @@ Socket::Socket(const Napi::CallbackInfo& info) assert(err == 0); socket = nullptr; - return; } Socket::~Socket() { @@ -217,8 +219,10 @@ 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."); } @@ -237,22 +241,23 @@ bool Socket::ValidateOpen() const { } bool Socket::HasEvents(int32_t requested) const { - int32_t events; + int32_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 0; +} } - return events & requested; + return (events & requested) != 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; @@ -283,7 +288,7 @@ 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; + uint32_t const flags = iter == end ? ZMQ_DONTWAIT : ZMQ_DONTWAIT | ZMQ_SNDMORE; while (zmq_msg_send(part, socket, flags) < 0) { if (zmq_errno() != EINTR) { res.Reject(ErrnoException(Env(), zmq_errno()).Value()); @@ -332,20 +337,23 @@ void Socket::Receive(const Napi::Promise::Deferred& res) { } #endif - if (!zmq_msg_more(part)) break; + if (zmq_msg_more(part) == 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,7 +361,7 @@ 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) { @@ -362,8 +370,8 @@ Napi::Value Socket::Bind(const Napi::CallbackInfo& info) { } } }, - [=]() { - AsyncScope scope(Env(), async_context); + [this, run_ctx, res]() { + AsyncScope const scope(Env(), async_context); state = Socket::State::Open; endpoints++; @@ -390,13 +398,15 @@ 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,7 +414,7 @@ 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) { @@ -413,8 +423,8 @@ Napi::Value Socket::Unbind(const Napi::CallbackInfo& info) { } } }, - [=]() { - AsyncScope scope(Env(), async_context); + [this, run_ctx, res]() { + AsyncScope const scope(Env(), async_context); state = Socket::State::Open; --endpoints; @@ -441,15 +451,17 @@ 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 +471,17 @@ 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 +491,8 @@ 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 +520,17 @@ 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 +585,8 @@ 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 +597,11 @@ 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 +632,8 @@ 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 { @@ -680,13 +701,14 @@ 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(); + uint32_t const option = info[0].As(); int32_t value = 0; size_t length = sizeof(value); @@ -695,22 +717,23 @@ 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,13 +742,14 @@ 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(); + uint32_t const option = info[0].As(); char value[1024]; size_t length = sizeof(value) - 1; @@ -736,38 +760,39 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { if (length == 0 || (length == 1 && value[0] == 0)) { return Env().Null(); - } else { - value[length] = '\0'; + } value[length] = '\0'; return Napi::String::New(Env(), value); - } + } 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 +805,14 @@ 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(); + uint32_t const option = info[0].As(); T value = 0; size_t length = sizeof(value); @@ -800,14 +826,15 @@ 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()); @@ -827,7 +854,7 @@ void Socket::SetSockOpt(const Napi::CallbackInfo& info) { } } -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 +863,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)); } @@ -897,7 +924,7 @@ void Socket::Poller::ReadableCallback() { assert(read_deferred); socket.sync_operations = 0; - AsyncScope scope(socket.Env(), socket.async_context); + AsyncScope const scope(socket.Env(), socket.async_context); socket.Receive(take(read_deferred)); } @@ -905,7 +932,7 @@ void Socket::Poller::WritableCallback() { assert(write_deferred); socket.sync_operations = 0; - AsyncScope scope(socket.Env(), socket.async_context); + AsyncScope const scope(socket.Env(), socket.async_context); socket.Send(take(write_deferred), write_value); write_value.Clear(); } @@ -924,4 +951,4 @@ Napi::Value Socket::Poller::WritePromise(OutgoingMsg::Parts&& value) { write_deferred = Napi::Promise::Deferred(socket.Env()); return write_deferred->Promise(); } -} +} // namespace zmq diff --git a/src/socket.h b/src/socket.h index 52d2ee89..62a05a57 100644 --- a/src/socket.h +++ b/src/socket.h @@ -15,7 +15,7 @@ class Socket : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Socket(const Napi::CallbackInfo& info); - virtual ~Socket(); + ~Socket() override; void Close() override; @@ -55,8 +55,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(int32_t 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 @@ -76,19 +76,19 @@ class Socket : public Napi::ObjectWrap, public Closable { 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 { + [[nodiscard]] bool ValidateReadable() const { return socket.HasEvents(ZMQ_POLLIN); } - inline bool ValidateWritable() const { + [[nodiscard]] bool ValidateWritable() const { return socket.HasEvents(ZMQ_POLLOUT); } @@ -117,7 +117,7 @@ class Socket : 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/util/arguments.h b/src/util/arguments.h index faa168a4..3d46fc31 100644 --- a/src/util/arguments.h +++ b/src/util/arguments.h @@ -6,9 +6,9 @@ #include "to_string.h" -namespace zmq { -namespace Arg { -typedef bool (Napi::Value::*ValueMethod)() const; +namespace zmq::Arg { + +using ValueMethod = bool (Napi::Value::*)() const; template struct VerifyWithMethod { @@ -33,11 +33,14 @@ class Verify { 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)); } }; @@ -64,10 +67,10 @@ class Validator { 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,13 +79,14 @@ 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 { + [[nodiscard]] 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" : ""); @@ -91,10 +95,12 @@ class Validator { 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..f14f6872 100644 --- a/src/util/electron_helper.h +++ b/src/util/electron_helper.h @@ -9,8 +9,8 @@ bool hasRun = false; 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); + std::string::size_type const pos = value.find('.'); + return pos == std::string::npos ? value : value.substr(0, pos); } /* Check if runtime is Electron. */ @@ -35,7 +35,7 @@ static inline bool hasElectronMemoryCage(const Napi::Env& env) { .Get("electron") .ToString() .Utf8Value(); - int majorVer = stoi(first_component(electronVers)); + int const majorVer = stoi(first_component(electronVers)); if (majorVer >= 21) { hasElectronMemoryCageCache = true; } @@ -44,4 +44,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..7a58e7ee 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -1,15 +1,15 @@ #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); +static constexpr const char* ErrnoMessage(int32_t errorno); +static constexpr const char* ErrnoCode(int32_t errorno); /* Generates a process warning message. */ static inline void Warn(const Napi::Env& env, const std::string& msg) { @@ -21,7 +21,7 @@ static inline void Warn(const Napi::Env& env, const std::string& msg) { static 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; @@ -29,7 +29,7 @@ static inline Napi::Error StatusException( static 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; @@ -38,8 +38,9 @@ static inline Napi::Error CodedException( /* This mostly duplicates node::ErrnoException, but it is not public. */ static 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; @@ -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) { +static constexpr const char* ErrnoMessage(int32_t errorno) { /* Clarify a few confusing default messages; otherwise rely on zmq. */ switch (errorno) { case EFAULT: @@ -78,7 +79,7 @@ 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) { +static constexpr const char* ErrnoCode(int32_t errorno) { #define ERRNO_CASE(e) \ case e: \ return #e; @@ -424,4 +425,4 @@ static inline constexpr const char* ErrnoCode(int32_t errorno) { return ""; } } -} +} // namespace zmq diff --git a/src/util/object.h b/src/util/object.h index 5093bb5c..46fca4d2 100644 --- a/src/util/object.h +++ b/src/util/object.h @@ -17,4 +17,4 @@ static inline void Assign(Napi::Object object, Napi::Object options) { 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..7ca85b52 100644 --- a/src/util/reaper.h +++ b/src/util/reaper.h @@ -32,12 +32,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 +49,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 index e1cba4ff..d75ef1f8 100644 --- a/src/util/to_string.h +++ b/src/util/to_string.h @@ -6,7 +6,9 @@ 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"; + if (val == 0) { + return "0"; + } std::string str; while (val > 0) { @@ -16,4 +18,4 @@ static inline std::string to_string(int64_t val) { return str; } -} +} // namespace zmq diff --git a/src/util/trash.h b/src/util/trash.h index 618bf334..dfc676de 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; @@ -34,12 +34,12 @@ class Trash { /* 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); } /* 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); + auto err = uv_async_send(this->async); 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..a70d7220 100644 --- a/src/util/uvdelayed.h +++ b/src/util/uvdelayed.h @@ -13,8 +13,8 @@ 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); @@ -25,8 +25,8 @@ class UvDelayed { 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) {}); @@ -50,4 +50,4 @@ static 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..0602f6c2 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,24 @@ 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->; + // NOLINTNEXTLINE(*-explicit-*) inline operator bool() { return handle_ptr::operator bool() && handle_ptr::get()->type != 0; } + // NOLINTNEXTLINE(*-explicit-*) inline operator T*() { return handle_ptr::get(); } + // NOLINTNEXTLINE(*-explicit-*) inline operator uv_handle_t*() { return reinterpret_cast(handle_ptr::get()); } }; -} +} // namespace zmq diff --git a/src/util/uvwork.h b/src/util/uvwork.h index a3eadda4..ad7c1226 100644 --- a/src/util/uvwork.h +++ b/src/util/uvwork.h @@ -14,25 +14,27 @@ 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; } @@ -40,8 +42,8 @@ class UvWork { template static inline int32_t UvQueue(const Napi::Env& env, E execute, C complete) { - auto loop = UvLoop(env); + auto* loop = UvLoop(env); auto work = new UvWork(std::move(execute), std::move(complete)); return work->Schedule(loop); } -} +} // namespace zmq From 864bebfb029dfa5e6454c39c643d09ec7c066c9f Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Mon, 21 Oct 2024 00:09:11 -0700 Subject: [PATCH 02/27] fix: set error/status as maybe unused --- src/context.cc | 2 +- src/incoming_msg.cc | 4 ++-- src/module.cc | 2 +- src/module.h | 2 +- src/observer.cc | 4 ++-- src/outgoing_msg.cc | 2 +- src/poller.h | 14 +++++++------- src/proxy.cc | 4 ++-- src/socket.cc | 8 ++++---- src/util/trash.h | 4 ++-- src/util/uvloop.h | 2 +- 11 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/context.cc b/src/context.cc index 20099f47..31db0253 100644 --- a/src/context.cc +++ b/src/context.cc @@ -66,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. */ diff --git a/src/incoming_msg.cc b/src/incoming_msg.cc index be417d6e..f3068f81 100644 --- a/src/incoming_msg.cc +++ b/src/incoming_msg.cc @@ -58,12 +58,12 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { } 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/module.cc b/src/module.cc index 86372dc0..ee4ceeec 100644 --- a/src/module.cc +++ b/src/module.cc @@ -118,7 +118,7 @@ NAPI_MODULE_INIT(/* 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 6f0ba707..38eb6697 100644 --- a/src/module.h +++ b/src/module.h @@ -27,7 +27,7 @@ struct Terminator { /* 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); }); diff --git a/src/observer.cc b/src/observer.cc index d6163c84..13e843e6 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -179,7 +179,7 @@ Observer::Observer(const Napi::CallbackInfo& info) return; error: - auto err = zmq_close(socket); + [[maybe_unused]] auto err =zmq_close(socket); assert(err == 0); socket = nullptr; @@ -219,7 +219,7 @@ void Observer::Close() { 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. */ diff --git a/src/outgoing_msg.cc b/src/outgoing_msg.cc index f8f269c3..68e46280 100644 --- a/src/outgoing_msg.cc +++ b/src/outgoing_msg.cc @@ -88,7 +88,7 @@ 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); } diff --git a/src/poller.h b/src/poller.h index 956e0b1c..70d28ebd 100644 --- a/src/poller.h +++ b/src/poller.h @@ -70,7 +70,7 @@ class Poller { assert((events & UV_READABLE) == 0); if (timeout > 0) { - auto err = uv_timer_start( + [[maybe_unused]] auto err =uv_timer_start( readable_timer, [](uv_timer_t* timer) { auto& poller = *reinterpret_cast(timer->data); @@ -83,7 +83,7 @@ class Poller { 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, UV_READABLE, Callback); assert(err == 0); } @@ -94,7 +94,7 @@ class Poller { assert((events & UV_WRITABLE) == 0); if (timeout > 0) { - auto err = uv_timer_start( + [[maybe_unused]] auto err =uv_timer_start( writable_timer, [](uv_timer_t* timer) { auto& poller = *reinterpret_cast(timer->data); @@ -109,7 +109,7 @@ class Poller { events on the socket in an edge-triggered fashion by making the file descriptor become ready for READING." */ if (events == 0) { - auto err = uv_poll_start(poll, UV_READABLE, Callback); + [[maybe_unused]] auto err =uv_poll_start(poll, UV_READABLE, Callback); assert(err == 0); } @@ -141,18 +141,18 @@ class Poller { void Trigger(int32_t triggered) { events &= ~triggered; if (events == 0) { - auto err = uv_poll_stop(poll); + [[maybe_unused]] auto err =uv_poll_stop(poll); assert(err == 0); } if ((triggered & UV_READABLE) != 0) { - auto err = uv_timer_stop(readable_timer); + [[maybe_unused]] auto err =uv_timer_stop(readable_timer); assert(err == 0); static_cast(this)->ReadableCallback(); } if ((triggered & UV_WRITABLE) != 0) { - auto err = uv_timer_stop(writable_timer); + [[maybe_unused]] auto err =uv_timer_stop(writable_timer); assert(err == 0); static_cast(this)->WritableCallback(); } diff --git a/src/proxy.cc b/src/proxy.cc index 3df2ceb6..f62fe1d3 100644 --- a/src/proxy.cc +++ b/src/proxy.cc @@ -131,10 +131,10 @@ Napi::Value Proxy::Run(const Napi::CallbackInfo& info) { 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; diff --git a/src/socket.cc b/src/socket.cc index 89e7397d..ca5d971d 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -128,7 +128,7 @@ Socket::Socket(const Napi::CallbackInfo& info) /* 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); }; @@ -177,7 +177,7 @@ Socket::Socket(const Napi::CallbackInfo& info) return; error: - auto err = zmq_close(socket); + [[maybe_unused]] auto err =zmq_close(socket); assert(err == 0); socket = nullptr; @@ -266,7 +266,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. */ @@ -762,7 +762,7 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { return Env().Null(); } value[length] = '\0'; return Napi::String::New(Env(), value); - + } template <> diff --git a/src/util/trash.h b/src/util/trash.h index dfc676de..7aeac0ee 100644 --- a/src/util/trash.h +++ b/src/util/trash.h @@ -29,7 +29,7 @@ class Trash { reinterpret_cast(async->data)->Clear(); }; - auto err = uv_async_init(loop, async, clear); + [[maybe_unused]] auto err =uv_async_init(loop, async, clear); assert(err == 0); /* Immediately unreference this handle in order to prevent the async @@ -47,7 +47,7 @@ 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(this->async); + [[maybe_unused]] auto err =uv_async_send(this->async); assert(err == 0); } 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; } From 2e50478008c4f62591ce52f28f28163c915aa91c Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Mon, 21 Oct 2024 00:12:46 -0700 Subject: [PATCH 03/27] fix: explicit fall through in switch --- src/outgoing_msg.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/outgoing_msg.cc b/src/outgoing_msg.cc index 68e46280..9b173c80 100644 --- a/src/outgoing_msg.cc +++ b/src/outgoing_msg.cc @@ -81,7 +81,7 @@ OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { } /* Fall through */ - default: + [[fallthrough]]; default: string_send(new std::string(value.ToString())); } } From 37889051ecfc9a4db8f0e6ccf3dd08b94a894d39 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Mon, 21 Oct 2024 00:15:38 -0700 Subject: [PATCH 04/27] fix: remove duplicate NOMINMAX --- src/socket.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/socket.cc b/src/socket.cc index ca5d971d..d0410751 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -1,7 +1,3 @@ - - -#define NOMINMAX // prevent minwindef.h from defining max macro in the debug build - #include "./socket.h" #include From cd7e43212611448789cf0d9f563ade7e767a373c Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Mon, 21 Oct 2024 00:26:30 -0700 Subject: [PATCH 05/27] fix: guard force inline behind not clang --- src/inline.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From 32d7ba0af3f803738bffee85e94c546c4a920a92 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Mon, 21 Oct 2024 00:27:55 -0700 Subject: [PATCH 06/27] fix: make closable's destructor virtual --- src/closable.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/closable.h b/src/closable.h index 891ebf7f..ca7a68c7 100644 --- a/src/closable.h +++ b/src/closable.h @@ -4,6 +4,7 @@ up ZMQ resources at agent exit. */ namespace zmq { struct Closable { + virtual ~Closable() = default; virtual void Close() = 0; }; } From 8eb8359ce827259ae1d1e3307f651fd2af2dd450 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Mon, 21 Oct 2024 00:42:47 -0700 Subject: [PATCH 07/27] fix: avoid sign conversion for options/errors/timeout --- src/context.cc | 10 +-- src/incoming_msg.cc | 7 +- src/module.h | 2 +- src/observer.cc | 4 +- src/outgoing_msg.cc | 5 +- src/poller.h | 19 +++-- src/proxy.cc | 9 +- src/socket.cc | 194 +++++++++++++++++++++++++------------------- src/util/trash.h | 4 +- 9 files changed, 145 insertions(+), 109 deletions(-) diff --git a/src/context.cc b/src/context.cc index 31db0253..91d1970a 100644 --- a/src/context.cc +++ b/src/context.cc @@ -66,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. */ - [[maybe_unused]] 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. */ @@ -88,7 +88,7 @@ Napi::Value Context::GetCtxOpt(const Napi::CallbackInfo& info) { return Env().Undefined(); } - uint32_t const option = info[0].As(); + const auto option = info[0].As(); int32_t const value = zmq_ctx_get(context, option); if (value < 0) { @@ -110,7 +110,7 @@ void Context::SetCtxOpt(const Napi::CallbackInfo& info) { return; } - uint32_t const option = info[0].As(); + const auto option = info[0].As(); int32_t const value = static_cast(info[1].As()); if (zmq_ctx_set(context, option, value) < 0) { @@ -129,7 +129,7 @@ Napi::Value Context::GetCtxOpt(const Napi::CallbackInfo& info) { return Env().Undefined(); } - uint32_t const option = info[0].As(); + const auto option = info[0].As(); T value = zmq_ctx_get(context, option); if (value < 0) { @@ -151,7 +151,7 @@ void Context::SetCtxOpt(const Napi::CallbackInfo& info) { return; } - uint32_t const option = info[0].As(); + const auto option = info[0].As(); T value = info[1].As(); if (zmq_ctx_set(context, option, value) < 0) { diff --git a/src/incoming_msg.cc b/src/incoming_msg.cc index f3068f81..bc1cdfaa 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" @@ -41,7 +42,7 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { Napi::MemoryManagement::AdjustExternalMemory(env, length); auto release = [](const Napi::Env& env, uint8_t*, Reference* ref) { - ptrdiff_t const length = zmq_msg_size(*ref); + const auto length = static_cast(zmq_msg_size(*ref)); Napi::MemoryManagement::AdjustExternalMemory(env, -length); delete ref; }; @@ -58,12 +59,12 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { } IncomingMsg::Reference::Reference() { - [[maybe_unused]] auto err =zmq_msg_init(&msg); + [[maybe_unused]] auto err = zmq_msg_init(&msg); assert(err == 0); } IncomingMsg::Reference::~Reference() { - [[maybe_unused]] auto err =zmq_msg_close(&msg); + [[maybe_unused]] auto err = zmq_msg_close(&msg); assert(err == 0); } } // namespace zmq diff --git a/src/module.h b/src/module.h index 38eb6697..16ecc77a 100644 --- a/src/module.h +++ b/src/module.h @@ -27,7 +27,7 @@ struct Terminator { /* 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, [&] { - [[maybe_unused]] auto err =zmq_ctx_term(context); + [[maybe_unused]] auto err = zmq_ctx_term(context); assert(err == 0); }); diff --git a/src/observer.cc b/src/observer.cc index 13e843e6..6b23f6a6 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -179,7 +179,7 @@ Observer::Observer(const Napi::CallbackInfo& info) return; error: - [[maybe_unused]] auto err =zmq_close(socket); + [[maybe_unused]] auto err = zmq_close(socket); assert(err == 0); socket = nullptr; @@ -219,7 +219,7 @@ void Observer::Close() { Napi::HandleScope const scope(Env()); /* Close succeeds unless socket is invalid. */ - [[maybe_unused]] auto err =zmq_close(socket); + [[maybe_unused]] auto err = zmq_close(socket); assert(err == 0); /* Reset pointer to avoid double close. */ diff --git a/src/outgoing_msg.cc b/src/outgoing_msg.cc index 9b173c80..951e405a 100644 --- a/src/outgoing_msg.cc +++ b/src/outgoing_msg.cc @@ -81,14 +81,15 @@ OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { } /* Fall through */ - [[fallthrough]]; default: + [[fallthrough]]; + default: string_send(new std::string(value.ToString())); } } } OutgoingMsg::~OutgoingMsg() { - [[maybe_unused]] auto err =zmq_msg_close(&msg); + [[maybe_unused]] auto err = zmq_msg_close(&msg); assert(err == 0); } diff --git a/src/poller.h b/src/poller.h index 70d28ebd..eec95246 100644 --- a/src/poller.h +++ b/src/poller.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "util/uvhandle.h" @@ -70,20 +71,20 @@ class Poller { assert((events & UV_READABLE) == 0); if (timeout > 0) { - [[maybe_unused]] auto err =uv_timer_start( + [[maybe_unused]] auto err = uv_timer_start( readable_timer, [](uv_timer_t* timer) { auto& poller = *reinterpret_cast(timer->data); poller.Trigger(UV_READABLE); }, - timeout, 0); + static_cast(timeout), 0); assert(err == 0); } if (events == 0) { /* Only start polling if we were not polling already. */ - [[maybe_unused]] auto err =uv_poll_start(poll, UV_READABLE, Callback); + [[maybe_unused]] auto err = uv_poll_start(poll, UV_READABLE, Callback); assert(err == 0); } @@ -94,13 +95,13 @@ class Poller { assert((events & UV_WRITABLE) == 0); if (timeout > 0) { - [[maybe_unused]] auto err =uv_timer_start( + [[maybe_unused]] auto err = uv_timer_start( writable_timer, [](uv_timer_t* timer) { auto& poller = *reinterpret_cast(timer->data); poller.Trigger(UV_WRITABLE); }, - timeout, 0); + static_cast(timeout), 0); assert(err == 0); } @@ -109,7 +110,7 @@ class Poller { events on the socket in an edge-triggered fashion by making the file descriptor become ready for READING." */ if (events == 0) { - [[maybe_unused]] auto err =uv_poll_start(poll, UV_READABLE, Callback); + [[maybe_unused]] auto err = uv_poll_start(poll, UV_READABLE, Callback); assert(err == 0); } @@ -141,18 +142,18 @@ class Poller { void Trigger(int32_t triggered) { events &= ~triggered; if (events == 0) { - [[maybe_unused]] auto err =uv_poll_stop(poll); + [[maybe_unused]] auto err = uv_poll_stop(poll); assert(err == 0); } if ((triggered & UV_READABLE) != 0) { - [[maybe_unused]] auto err =uv_timer_stop(readable_timer); + [[maybe_unused]] auto err = uv_timer_stop(readable_timer); assert(err == 0); static_cast(this)->ReadableCallback(); } if ((triggered & UV_WRITABLE) != 0) { - [[maybe_unused]] auto err =uv_timer_stop(writable_timer); + [[maybe_unused]] auto err = uv_timer_stop(writable_timer); assert(err == 0); static_cast(this)->WritableCallback(); } diff --git a/src/proxy.cc b/src/proxy.cc index f62fe1d3..cf43fe05 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" @@ -116,12 +118,12 @@ Napi::Value Proxy::Run(const Napi::CallbackInfo& info) { [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; } }, @@ -141,7 +143,8 @@ Napi::Value Proxy::Run(const Napi::CallbackInfo& info) { 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; } diff --git a/src/socket.cc b/src/socket.cc index d0410751..4ae77316 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -1,6 +1,7 @@ #include "./socket.h" #include +#include #include #include @@ -33,8 +34,9 @@ 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)) { Warn(num.Env(), @@ -67,8 +69,9 @@ Socket::Socket(const Napi::CallbackInfo& info) Arg::Optional("Options must be an object"), }; - if (args.ThrowIfInvalid(info)) { return; -} + if (args.ThrowIfInvalid(info)) { + return; + } type = info[0].As().Uint32Value(); @@ -84,9 +87,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) { @@ -124,7 +128,7 @@ Socket::Socket(const Napi::CallbackInfo& info) /* Callback to free the underlying poller. Move the poller to transfer ownership after the constructor has completed. */ finalize = [=]() mutable { - [[maybe_unused]] auto err =zmq_poller_destroy(&poll); + [[maybe_unused]] auto err = zmq_poller_destroy(&poll); assert(err == 0); }; @@ -173,7 +177,7 @@ Socket::Socket(const Napi::CallbackInfo& info) return; error: - [[maybe_unused]] auto err =zmq_close(socket); + [[maybe_unused]] auto err = zmq_close(socket); assert(err == 0); socket = nullptr; @@ -215,10 +219,12 @@ void Socket::WarnUnlessImmediateOption(int32_t option) const { ZMQ_RCVTIMEO, }; - if (immediate.contains(option)) { 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."); } @@ -242,8 +248,9 @@ bool Socket::HasEvents(int32_t requested) const { 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) != 0; @@ -262,7 +269,7 @@ void Socket::Close() { poller.Close(); /* Close succeeds unless socket is invalid. */ - [[maybe_unused]] auto err =zmq_close(socket); + [[maybe_unused]] auto err = zmq_close(socket); assert(err == 0); /* Release reference to context and observer. */ @@ -284,7 +291,7 @@ void Socket::Send(const Napi::Promise::Deferred& res, OutgoingMsg::Parts& parts) auto& part = *iter; iter++; - uint32_t const flags = iter == end ? ZMQ_DONTWAIT : ZMQ_DONTWAIT | ZMQ_SNDMORE; + auto const flags = iter == end ? ZMQ_DONTWAIT : ZMQ_DONTWAIT | ZMQ_SNDMORE; while (zmq_msg_send(part, socket, flags) < 0) { if (zmq_errno() != EINTR) { res.Reject(ErrnoException(Env(), zmq_errno()).Value()); @@ -333,8 +340,9 @@ void Socket::Receive(const Napi::Promise::Deferred& res) { } #endif - if (zmq_msg_more(part) == 0) { break; -} + if (zmq_msg_more(part) == 0) { + break; + } } res.Resolve(list); @@ -345,11 +353,13 @@ Napi::Value Socket::Bind(const Napi::CallbackInfo& info) { 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()); @@ -361,7 +371,7 @@ Napi::Value Socket::Bind(const Napi::CallbackInfo& info) { /* 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; } } @@ -377,8 +387,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; } @@ -398,11 +409,13 @@ Napi::Value Socket::Unbind(const Napi::CallbackInfo& info) { 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()); @@ -414,7 +427,7 @@ Napi::Value Socket::Unbind(const Napi::CallbackInfo& info) { /* 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; } } @@ -430,8 +443,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; } @@ -451,11 +465,13 @@ void Socket::Connect(const Napi::CallbackInfo& info) { 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 const address = info[0].As(); if (zmq_connect(socket, address.c_str()) < 0) { @@ -471,11 +487,13 @@ void Socket::Disconnect(const Napi::CallbackInfo& info) { 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 const address = info[0].As(); if (zmq_disconnect(socket, address.c_str()) < 0) { @@ -487,8 +505,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) { @@ -520,13 +539,15 @@ Napi::Value Socket::Send(const Napi::CallbackInfo& info) { 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, @@ -581,8 +602,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 { @@ -593,11 +615,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, @@ -628,8 +652,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 { @@ -639,7 +664,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"), @@ -667,7 +692,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"), @@ -701,10 +726,11 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { Arg::Required("Identifier must be a number"), }; - if (args.ThrowIfInvalid(info)) { return Env().Undefined(); -} + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - uint32_t const option = info[0].As(); + auto const option = info[0].As(); int32_t value = 0; size_t length = sizeof(value); @@ -723,8 +749,9 @@ void Socket::SetSockOpt(const Napi::CallbackInfo& info) { Arg::Required("Option value must be a boolean"), }; - if (args.ThrowIfInvalid(info)) { return; -} + if (args.ThrowIfInvalid(info)) { + return; + } int32_t const option = info[0].As(); WarnUnlessImmediateOption(option); @@ -742,10 +769,11 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { Arg::Required("Identifier must be a number"), }; - if (args.ThrowIfInvalid(info)) { return Env().Undefined(); -} + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - uint32_t const option = info[0].As(); + auto const option = info[0].As(); char value[1024]; size_t length = sizeof(value) - 1; @@ -756,9 +784,9 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { if (length == 0 || (length == 1 && value[0] == 0)) { return Env().Null(); - } value[length] = '\0'; - return Napi::String::New(Env(), value); - + } + value[length] = '\0'; + return Napi::String::New(Env(), value); } template <> @@ -780,12 +808,12 @@ void Socket::SetSockOpt(const Napi::CallbackInfo& info) { if (info[1].IsBuffer()) { 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; @@ -805,10 +833,11 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { Arg::Required("Identifier must be a number"), }; - if (args.ThrowIfInvalid(info)) { return Env().Undefined(); -} + if (args.ThrowIfInvalid(info)) { + return Env().Undefined(); + } - uint32_t const option = info[0].As(); + auto const option = info[0].As(); T value = 0; size_t length = sizeof(value); @@ -827,8 +856,9 @@ void Socket::SetSockOpt(const Napi::CallbackInfo& info) { Arg::Required("Option value must be a number"), }; - if (args.ThrowIfInvalid(info)) { return; -} + if (args.ThrowIfInvalid(info)) { + return; + } int32_t const option = info[0].As(); WarnUnlessImmediateOption(option); @@ -850,7 +880,7 @@ void Socket::SetSockOpt(const Napi::CallbackInfo& info) { } } -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); @@ -859,19 +889,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)); } @@ -940,10 +970,10 @@ Napi::Value Socket::Poller::ReadPromise() { 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_value = std::move(parts); write_deferred = Napi::Promise::Deferred(socket.Env()); return write_deferred->Promise(); } diff --git a/src/util/trash.h b/src/util/trash.h index 7aeac0ee..a487ac06 100644 --- a/src/util/trash.h +++ b/src/util/trash.h @@ -29,7 +29,7 @@ class Trash { reinterpret_cast(async->data)->Clear(); }; - [[maybe_unused]] auto err =uv_async_init(loop, async, clear); + [[maybe_unused]] auto err = uv_async_init(loop, async, clear); assert(err == 0); /* Immediately unreference this handle in order to prevent the async @@ -47,7 +47,7 @@ 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. */ - [[maybe_unused]] auto err =uv_async_send(this->async); + [[maybe_unused]] auto err = uv_async_send(this->async); assert(err == 0); } From a83f66582f27926fe33a7e9c69951d73774015d1 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Mon, 21 Oct 2024 00:53:07 -0700 Subject: [PATCH 08/27] fix: set global variables as static --- src/util/electron_helper.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/electron_helper.h b/src/util/electron_helper.h index f14f6872..eebeba4c 100644 --- a/src/util/electron_helper.h +++ b/src/util/electron_helper.h @@ -5,8 +5,8 @@ #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 const pos = value.find('.'); From e4efd54a7cb71c30263975ade80211872a286634 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 22 Oct 2024 01:37:40 -0700 Subject: [PATCH 09/27] fix: fix rest of sign conversions for timeout/error --- src/incoming_msg.cc | 3 ++- src/observer.cc | 6 +++++- src/socket.cc | 6 ++++-- src/util/uvdelayed.h | 2 +- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/incoming_msg.cc b/src/incoming_msg.cc index bc1cdfaa..0261e331 100644 --- a/src/incoming_msg.cc +++ b/src/incoming_msg.cc @@ -39,7 +39,8 @@ 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) { const auto length = static_cast(zmq_msg_size(*ref)); diff --git a/src/observer.cc b/src/observer.cc index 6b23f6a6..01288dd1 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" @@ -283,7 +285,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 @@ -305,6 +307,8 @@ void Observer::Receive(const Napi::Promise::Deferred& res) { Close(); break; } + default: + break; } res.Resolve(event); diff --git a/src/socket.cc b/src/socket.cc index 4ae77316..438db82b 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -872,10 +872,12 @@ 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; } } diff --git a/src/util/uvdelayed.h b/src/util/uvdelayed.h index a70d7220..3ac90943 100644 --- a/src/util/uvdelayed.h +++ b/src/util/uvdelayed.h @@ -29,7 +29,7 @@ class UvDelayed { [[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, []([[maybe_unused]] uv_idle_t* idle) {}); assert(err == 0); err = uv_check_start(check, [](uv_check_t* check) { From 3288ea05aaeaf6701341a57d1829aa2fcbf2c988 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 22 Oct 2024 01:59:25 -0700 Subject: [PATCH 10/27] build: add debug mode for linux --- package.json | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 8549e6a7..bb80e2bb 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", @@ -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", From 9546fcc817dbf10e7b81fef80d6c0e4c2430950a Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 22 Oct 2024 02:07:02 -0700 Subject: [PATCH 11/27] fix: avoid conversion issue for max double limits --- src/socket.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/socket.cc b/src/socket.cc index 438db82b..b57885d8 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -38,7 +38,8 @@ uint64_t NumberCast(const Napi::Number& num) { return 0; } - if (value > static_cast((1ULL << 53) - 1)) { + static constexpr auto max_safe_integer = static_cast((1ULL << 53) - 1); + if (value > max_safe_integer) { Warn(num.Env(), "Value is larger than Number.MAX_SAFE_INTEGER and may have been rounded " "inaccurately."); @@ -46,8 +47,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(); } From 0beb8d3a012a46e754300dd91637e5964fd78df8 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 22 Oct 2024 02:08:40 -0700 Subject: [PATCH 12/27] fix: use int32_t for the socket type --- src/socket.cc | 2 +- src/socket.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/socket.cc b/src/socket.cc index b57885d8..fd7831bf 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -76,7 +76,7 @@ Socket::Socket(const Napi::CallbackInfo& info) return; } - type = info[0].As().Uint32Value(); + type = info[0].As().Int32Value(); if (info[1].IsObject()) { auto options = info[1].As(); diff --git a/src/socket.h b/src/socket.h index 62a05a57..fdc46abb 100644 --- a/src/socket.h +++ b/src/socket.h @@ -112,7 +112,7 @@ 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; From a3feede8982f4f6bb7912d21bf5d1404de2ab29f Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 22 Oct 2024 02:12:23 -0700 Subject: [PATCH 13/27] fix: remove to_string override for int64_t --- src/module.cc | 6 +++--- src/observer.cc | 2 +- src/proxy.cc | 2 +- src/util/arguments.h | 5 ++--- src/util/to_string.h | 21 --------------------- 5 files changed, 7 insertions(+), 29 deletions(-) delete mode 100644 src/util/to_string.h diff --git a/src/module.cc b/src/module.cc index ee4ceeec..ff939feb 100644 --- a/src/module.cc +++ b/src/module.cc @@ -8,7 +8,6 @@ #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) { @@ -17,8 +16,9 @@ static inline Napi::String Version(const Napi::Env& env) { 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) { diff --git a/src/observer.cc b/src/observer.cc index 01288dd1..f595ebc2 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -143,7 +143,7 @@ Observer::Observer(const Napi::CallbackInfo& info) /* 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(); diff --git a/src/proxy.cc b/src/proxy.cc index cf43fe05..520f2519 100644 --- a/src/proxy.cc +++ b/src/proxy.cc @@ -96,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) { diff --git a/src/util/arguments.h b/src/util/arguments.h index 3d46fc31..e34359f7 100644 --- a/src/util/arguments.h +++ b/src/util/arguments.h @@ -4,8 +4,6 @@ #include -#include "to_string.h" - namespace zmq::Arg { using ValueMethod = bool (Napi::Value::*)() const; @@ -89,7 +87,8 @@ class Validator { [[nodiscard]] 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" : ""); + auto msg = + "Expected " + std::to_string(N) + " argument" + (N != 1 ? "s" : ""); return Napi::TypeError::New(info.Env(), msg); } diff --git a/src/util/to_string.h b/src/util/to_string.h deleted file mode 100644 index d75ef1f8..00000000 --- a/src/util/to_string.h +++ /dev/null @@ -1,21 +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; -} -} // namespace zmq From ac41cdd8b4e1399f2805b84ea190b636bd8f467d Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 22 Oct 2024 02:19:13 -0700 Subject: [PATCH 14/27] test: retry flaky tests + add more timeout --- .mocharc.js | 2 ++ 1 file changed, 2 insertions(+) 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 From e1f0804bb4beafb944dfe9b69a487e9f0fa3e41d Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 04:42:24 -0700 Subject: [PATCH 15/27] fix: disable -Wshadow on gcc + fix useless cast warning --- CMakeLists.txt | 6 +++++- src/outgoing_msg.cc | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 710b76b9..fe2dce4b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -102,7 +102,11 @@ 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) + 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/src/outgoing_msg.cc b/src/outgoing_msg.cc index 951e405a..3ad47fb4 100644 --- a/src/outgoing_msg.cc +++ b/src/outgoing_msg.cc @@ -44,7 +44,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); From 522537bd92e3c1d4416352a4972bbd4f290d8b80 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 04:50:02 -0700 Subject: [PATCH 16/27] fix: remove unnecessary static/inline keywords f --- .github/workflows/CI.yml | 3 ++- package.json | 2 +- src/module.cc | 6 +++--- src/observer.cc | 14 +++++++------- src/socket.cc | 8 ++++---- src/util/electron_helper.h | 6 +++--- src/util/error.h | 14 +++++++------- src/util/object.h | 4 ++-- src/util/uvdelayed.h | 2 +- src/util/uvwork.h | 2 +- 10 files changed, 31 insertions(+), 30 deletions(-) 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/package.json b/package.json index bb80e2bb..1c74d845 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/module.cc b/src/module.cc index ff939feb..68491252 100644 --- a/src/module.cc +++ b/src/module.cc @@ -10,7 +10,7 @@ #include "util/error.h" namespace zmq { -static inline Napi::String Version(const Napi::Env& env) { +Napi::String Version(const Napi::Env& env) { int32_t major = 0; int32_t minor = 0; int32_t patch = 0; @@ -21,7 +21,7 @@ static inline Napi::String Version(const Napi::Env& env) { + 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 @@ -57,7 +57,7 @@ static inline Napi::Object Capabilities(const Napi::Env& env) { return result; } -static inline Napi::Value CurveKeyPair(const Napi::CallbackInfo& info) { +Napi::Value CurveKeyPair(const Napi::CallbackInfo& info) { char public_key[41]; char secret_key[41]; if (zmq_curve_keypair(public_key, secret_key) < 0) { diff --git a/src/observer.cc b/src/observer.cc index f595ebc2..486adb8b 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -12,7 +12,7 @@ #include "util/take.h" namespace zmq { -static constexpr const char* EventName(uint32_t val) { +constexpr const char* EventName(uint32_t val) { switch (val) { case ZMQ_EVENT_CONNECTED: return "connect"; @@ -76,7 +76,7 @@ static constexpr const char* EventName(uint32_t val) { } #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_AUTH -static constexpr const char* AuthError(uint32_t val) { +constexpr const char* AuthError(uint32_t val) { switch (val) { case 300: return "Temporary error"; @@ -92,7 +92,7 @@ static constexpr const char* AuthError(uint32_t val) { #endif #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL -static inline std::pair ProtoError(uint32_t val) { +std::pair ProtoError(uint32_t val) { #define PROTO_ERROR_CASE(_prefix, _err) \ case ZMQ_PROTOCOL_ERROR_##_prefix##_##_err: \ return std::make_pair(#_prefix " protocol error", "ERR_" #_prefix "_" #_err); @@ -157,20 +157,20 @@ Observer::Observer(const Napi::CallbackInfo& info) return; } - uv_os_sock_t fd = 0; - size_t length = sizeof(fd); + uv_os_sock_t file_descriptor = 0; + size_t length = sizeof(file_descriptor); if (zmq_connect(socket, address.c_str()) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); goto 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; } - if (poller.Initialize(Env(), fd) < 0) { + if (poller.Initialize(Env(), file_descriptor) < 0) { ErrnoException(Env(), errno).ThrowAsJavaScriptException(); goto error; } diff --git a/src/socket.cc b/src/socket.cc index fd7831bf..1e44be7a 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -101,7 +101,7 @@ Socket::Socket(const Napi::CallbackInfo& info) return; } - uv_os_sock_t fd = 0; + uv_os_sock_t file_descriptor = 0; std::function const finalize = nullptr; #ifdef ZMQ_THREAD_SAFE @@ -153,14 +153,14 @@ Socket::Socket(const Napi::CallbackInfo& info) goto 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; } } - if (poller.Initialize(Env(), fd, finalize) < 0) { + if (poller.Initialize(Env(), file_descriptor, finalize) < 0) { ErrnoException(Env(), errno).ThrowAsJavaScriptException(); goto error; } diff --git a/src/util/electron_helper.h b/src/util/electron_helper.h index eebeba4c..2520ab2f 100644 --- a/src/util/electron_helper.h +++ b/src/util/electron_helper.h @@ -8,13 +8,13 @@ namespace zmq { static bool hasRun = false; static bool hasElectronMemoryCageCache = false; -static inline std::string first_component(std::string const& value) { +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() diff --git a/src/util/error.h b/src/util/error.h index 7a58e7ee..c63db269 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -12,14 +12,14 @@ static constexpr const char* ErrnoMessage(int32_t errorno); static 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 const scope(env); auto exception = Napi::Error::New(env, msg); @@ -27,7 +27,7 @@ static inline Napi::Error StatusException( 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 const scope(env); auto exception = Napi::Error::New(env, msg); @@ -36,7 +36,7 @@ static inline Napi::Error CodedException( } /* 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 const scope(env); auto exception = @@ -46,7 +46,7 @@ static inline Napi::Error ErrnoException( 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)); diff --git a/src/util/object.h b/src/util/object.h index 46fca4d2..70c26379 100644 --- a/src/util/object.h +++ b/src/util/object.h @@ -4,14 +4,14 @@ 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(); diff --git a/src/util/uvdelayed.h b/src/util/uvdelayed.h index 3ac90943..f2312ed2 100644 --- a/src/util/uvdelayed.h +++ b/src/util/uvdelayed.h @@ -46,7 +46,7 @@ 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(); } diff --git a/src/util/uvwork.h b/src/util/uvwork.h index ad7c1226..801c2de3 100644 --- a/src/util/uvwork.h +++ b/src/util/uvwork.h @@ -41,7 +41,7 @@ class UvWork { }; template -static inline int32_t UvQueue(const Napi::Env& env, E execute, C complete) { +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); From d5f8199b1ed0df1480883fc36c6552594051f577 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 04:59:06 -0700 Subject: [PATCH 17/27] fix: avoid bitwise operations on integers --- src/incoming_msg.cc | 2 +- src/observer.cc | 6 ++++-- src/outgoing_msg.cc | 2 +- src/poller.h | 4 ++-- src/socket.cc | 10 +++++----- src/socket.h | 2 +- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/incoming_msg.cc b/src/incoming_msg.cc index 0261e331..64844954 100644 --- a/src/incoming_msg.cc +++ b/src/incoming_msg.cc @@ -31,7 +31,7 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { auto length = zmq_msg_size(*ref); 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 diff --git a/src/observer.cc b/src/observer.cc index 486adb8b..95b5b300 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -77,6 +77,7 @@ constexpr const char* EventName(uint32_t val) { #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_AUTH constexpr const char* AuthError(uint32_t val) { + // NOLINTBEGIN(*-magic-numbers) switch (val) { case 300: return "Temporary error"; @@ -88,6 +89,7 @@ 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 @@ -201,13 +203,13 @@ bool Observer::ValidateOpen() const { } bool Observer::HasEvents() const { - int32_t events = 0; + 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; + return false; } } diff --git a/src/outgoing_msg.cc b/src/outgoing_msg.cc index 3ad47fb4..87ca0904 100644 --- a/src/outgoing_msg.cc +++ b/src/outgoing_msg.cc @@ -6,7 +6,7 @@ namespace zmq { OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { - static auto constexpr zero_copy_threshold = 1 << 7; + 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 diff --git a/src/poller.h b/src/poller.h index eec95246..523a42b6 100644 --- a/src/poller.h +++ b/src/poller.h @@ -16,7 +16,7 @@ class Poller { UvHandle readable_timer; UvHandle writable_timer; - int32_t events{0}; + uint32_t events{0}; std::function finalize = nullptr; public: @@ -139,7 +139,7 @@ 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. */ - void Trigger(int32_t triggered) { + void Trigger(uint32_t triggered) { events &= ~triggered; if (events == 0) { [[maybe_unused]] auto err = uv_poll_stop(poll); diff --git a/src/socket.cc b/src/socket.cc index 1e44be7a..21d6b284 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -20,7 +20,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 @@ -38,7 +38,7 @@ uint64_t NumberCast(const Napi::Number& num) { return 0; } - static constexpr auto max_safe_integer = 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 " @@ -245,8 +245,8 @@ bool Socket::ValidateOpen() const { } } -bool Socket::HasEvents(int32_t requested) const { - int32_t events = 0; +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) { @@ -256,7 +256,7 @@ bool Socket::HasEvents(int32_t requested) const { } } - return (events & requested) != 0; + return (events & requested_events) != 0; } void Socket::Close() { diff --git a/src/socket.h b/src/socket.h index fdc46abb..c43c062a 100644 --- a/src/socket.h +++ b/src/socket.h @@ -56,7 +56,7 @@ class Socket : public Napi::ObjectWrap, public Closable { private: inline void WarnUnlessImmediateOption(int32_t option) const; [[nodiscard]] inline bool ValidateOpen() const; - [[nodiscard]] bool HasEvents(int32_t events) 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 From bae1d2fdcd7a0dbc51b41fd0335dbd5d08aa8a6f Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 05:04:11 -0700 Subject: [PATCH 18/27] fix: use explicit conversions for msg pointers --- src/incoming_msg.cc | 6 +++--- src/incoming_msg.h | 8 +++----- src/outgoing_msg.h | 3 +-- src/socket.cc | 6 +++--- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/incoming_msg.cc b/src/incoming_msg.cc index 64844954..7ae55d62 100644 --- a/src/incoming_msg.cc +++ b/src/incoming_msg.cc @@ -27,8 +27,8 @@ 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 = 1U << 7U; @@ -43,7 +43,7 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { env, static_cast(length)); auto release = [](const Napi::Env& env, uint8_t*, Reference* ref) { - const auto length = static_cast(zmq_msg_size(*ref)); + const auto length = static_cast(zmq_msg_size(ref->get())); Napi::MemoryManagement::AdjustExternalMemory(env, -length); delete ref; }; diff --git a/src/incoming_msg.h b/src/incoming_msg.h index f3a33903..d6df666b 100644 --- a/src/incoming_msg.h +++ b/src/incoming_msg.h @@ -15,9 +15,8 @@ class IncomingMsg { Napi::Value IntoBuffer(const Napi::Env& env); - // NOLINTNEXTLINE(*-explicit-*) - inline operator zmq_msg_t*() { - return *ref; + zmq_msg_t* get() { + return ref->get(); } private: @@ -28,8 +27,7 @@ class IncomingMsg { Reference(); ~Reference(); - // NOLINTNEXTLINE(*-explicit-*) - inline operator zmq_msg_t*() { + zmq_msg_t* get() { return &msg; } }; diff --git a/src/outgoing_msg.h b/src/outgoing_msg.h index 4012a35d..c83f5d21 100644 --- a/src/outgoing_msg.h +++ b/src/outgoing_msg.h @@ -24,8 +24,7 @@ class OutgoingMsg { explicit OutgoingMsg(Napi::Value value, Module& module); ~OutgoingMsg(); - // NOLINTNEXTLINE(*-explicit-*) - inline operator zmq_msg_t*() { + zmq_msg_t* get() { return &msg; } diff --git a/src/socket.cc b/src/socket.cc index 21d6b284..c2e1714e 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -295,7 +295,7 @@ void Socket::Send(const Napi::Promise::Deferred& res, OutgoingMsg::Parts& parts) iter++; auto const flags = iter == end ? ZMQ_DONTWAIT : ZMQ_DONTWAIT | ZMQ_SNDMORE; - while (zmq_msg_send(part, socket, flags) < 0) { + while (zmq_msg_send(part.get(), socket, flags) < 0) { if (zmq_errno() != EINTR) { res.Reject(ErrnoException(Env(), zmq_errno()).Value()); return; @@ -314,7 +314,7 @@ void Socket::Receive(const Napi::Promise::Deferred& res) { uint32_t i = 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; @@ -343,7 +343,7 @@ void Socket::Receive(const Napi::Promise::Deferred& res) { } #endif - if (zmq_msg_more(part) == 0) { + if (zmq_msg_more(part.get()) == 0) { break; } } From cf4ee12a32efd02bb5414e7c85ea19da45857c3d Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 05:12:15 -0700 Subject: [PATCH 19/27] fix: explicit conversions for getting uv handles --- src/poller.h | 20 ++++++++++---------- src/util/trash.h | 6 +++--- src/util/uvdelayed.h | 8 ++++---- src/util/uvhandle.h | 9 +++------ 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/poller.h b/src/poller.h index 523a42b6..e1b1391e 100644 --- a/src/poller.h +++ b/src/poller.h @@ -28,17 +28,17 @@ class Poller { 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(), fd); 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; } @@ -72,7 +72,7 @@ class Poller { if (timeout > 0) { [[maybe_unused]] auto err = uv_timer_start( - readable_timer, + readable_timer.get(), [](uv_timer_t* timer) { auto& poller = *reinterpret_cast(timer->data); poller.Trigger(UV_READABLE); @@ -84,7 +84,7 @@ class Poller { if (events == 0) { /* Only start polling if we were not polling already. */ - [[maybe_unused]] auto err = uv_poll_start(poll, UV_READABLE, Callback); + [[maybe_unused]] auto err = uv_poll_start(poll.get(), UV_READABLE, Callback); assert(err == 0); } @@ -96,7 +96,7 @@ class Poller { if (timeout > 0) { [[maybe_unused]] auto err = uv_timer_start( - writable_timer, + writable_timer.get(), [](uv_timer_t* timer) { auto& poller = *reinterpret_cast(timer->data); poller.Trigger(UV_WRITABLE); @@ -110,7 +110,7 @@ class Poller { events on the socket in an edge-triggered fashion by making the file descriptor become ready for READING." */ if (events == 0) { - [[maybe_unused]] auto err = uv_poll_start(poll, UV_READABLE, Callback); + [[maybe_unused]] auto err = uv_poll_start(poll.get(), UV_READABLE, Callback); assert(err == 0); } @@ -142,18 +142,18 @@ class Poller { void Trigger(uint32_t triggered) { events &= ~triggered; if (events == 0) { - [[maybe_unused]] auto err = uv_poll_stop(poll); + [[maybe_unused]] auto err = uv_poll_stop(poll.get()); assert(err == 0); } if ((triggered & UV_READABLE) != 0) { - [[maybe_unused]] auto err = uv_timer_stop(readable_timer); + [[maybe_unused]] auto err = uv_timer_stop(readable_timer.get()); assert(err == 0); static_cast(this)->ReadableCallback(); } if ((triggered & UV_WRITABLE) != 0) { - [[maybe_unused]] auto err = uv_timer_stop(writable_timer); + [[maybe_unused]] auto err = uv_timer_stop(writable_timer.get()); assert(err == 0); static_cast(this)->WritableCallback(); } diff --git a/src/util/trash.h b/src/util/trash.h index a487ac06..eff3e80c 100644 --- a/src/util/trash.h +++ b/src/util/trash.h @@ -29,12 +29,12 @@ class Trash { reinterpret_cast(async->data)->Clear(); }; - [[maybe_unused]] 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(this->async); + uv_unref(this->async.get_handle()); } /* Add given item to the trash, marking it for deletion next time the @@ -47,7 +47,7 @@ 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. */ - [[maybe_unused]] auto err = uv_async_send(this->async); + [[maybe_unused]] auto err = uv_async_send(this->async.get()); assert(err == 0); } diff --git a/src/util/uvdelayed.h b/src/util/uvdelayed.h index f2312ed2..f30ff376 100644 --- a/src/util/uvdelayed.h +++ b/src/util/uvdelayed.h @@ -17,11 +17,11 @@ class UvDelayed { [[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); } @@ -29,10 +29,10 @@ class UvDelayed { [[maybe_unused]] int32_t err = 0; /* Idle handle is needed to stop the event loop from blocking in poll. */ - err = uv_idle_start(idle, []([[maybe_unused]] 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(); diff --git a/src/util/uvhandle.h b/src/util/uvhandle.h index 0602f6c2..c7c641b9 100644 --- a/src/util/uvhandle.h +++ b/src/util/uvhandle.h @@ -35,18 +35,15 @@ class UvHandle : handle_ptr { using handle_ptr::reset; using handle_ptr::operator->; - // NOLINTNEXTLINE(*-explicit-*) - inline operator bool() { + explicit operator bool() { return handle_ptr::operator bool() && handle_ptr::get()->type != 0; } - // NOLINTNEXTLINE(*-explicit-*) - inline operator T*() { + T* get() { return handle_ptr::get(); } - // NOLINTNEXTLINE(*-explicit-*) - inline operator uv_handle_t*() { + uv_handle_t* get_handle() { return reinterpret_cast(handle_ptr::get()); } }; From 363613dfb42fd48dff1e6ba7013c5eeaf5708459 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 05:24:33 -0700 Subject: [PATCH 20/27] fix: add missing special functions for classes with destructors --- src/closable.h | 8 +++++++- src/context.h | 4 +++- src/incoming_msg.h | 6 ++++++ src/observer.h | 5 +++++ src/outgoing_msg.h | 2 ++ src/proxy.h | 5 +++++ src/socket.h | 5 +++++ src/util/reaper.h | 6 ++++++ 8 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/closable.h b/src/closable.h index ca7a68c7..bf9eb403 100644 --- a/src/closable.h +++ b/src/closable.h @@ -4,7 +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.h b/src/context.h index 9f637b93..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); - ~Context() override; + Context(const Context&) = delete; + Context& operator=(const Context&) = delete; Context(Context&&) = delete; Context& operator=(Context&&) = delete; + ~Context() override; void Close() override; diff --git a/src/incoming_msg.h b/src/incoming_msg.h index d6df666b..6d1252d9 100644 --- a/src/incoming_msg.h +++ b/src/incoming_msg.h @@ -12,6 +12,8 @@ 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); @@ -25,6 +27,10 @@ class IncomingMsg { public: Reference(); + Reference(const Reference&) = delete; + Reference(Reference&&) = delete; + Reference& operator=(const Reference&) = delete; + Reference& operator=(Reference&&) = delete; ~Reference(); zmq_msg_t* get() { diff --git a/src/observer.h b/src/observer.h index 84ba9b5a..54dbd7b4 100644 --- a/src/observer.h +++ b/src/observer.h @@ -16,6 +16,11 @@ class Observer : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Observer(const Napi::CallbackInfo& info); + + Observer(const Observer&) = delete; + Observer(Observer&&) = delete; + Observer& operator=(const Observer&) = delete; + Observer& operator=(Observer&&) = delete; ~Observer() override; void Close() override; diff --git a/src/outgoing_msg.h b/src/outgoing_msg.h index c83f5d21..e0f3170f 100644 --- a/src/outgoing_msg.h +++ b/src/outgoing_msg.h @@ -17,6 +17,8 @@ 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 diff --git a/src/proxy.h b/src/proxy.h index 9436bf97..dd7a01c6 100644 --- a/src/proxy.h +++ b/src/proxy.h @@ -15,6 +15,11 @@ class Proxy : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Proxy(const Napi::CallbackInfo& info); + + Proxy(const Proxy&) = delete; + Proxy(Proxy&&) = delete; + Proxy& operator=(const Proxy&) = delete; + Proxy& operator=(Proxy&&) = delete; ~Proxy() override; void Close() override; diff --git a/src/socket.h b/src/socket.h index c43c062a..04321511 100644 --- a/src/socket.h +++ b/src/socket.h @@ -15,6 +15,11 @@ class Socket : public Napi::ObjectWrap, public Closable { static void Initialize(Module& module, Napi::Object& exports); explicit Socket(const Napi::CallbackInfo& info); + + Socket(const Socket&) = delete; + Socket(Socket&&) = delete; + Socket& operator=(const Socket&) = delete; + Socket& operator=(Socket&&) = delete; ~Socket() override; void Close() override; diff --git a/src/util/reaper.h b/src/util/reaper.h index 7ca85b52..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 From f1fb6b2835f94cdf83b4fb6ae641d42ee9444172 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 05:44:15 -0700 Subject: [PATCH 21/27] fix: use reference wrappers as ref data members --- src/observer.cc | 6 +++--- src/observer.h | 7 ++++--- src/outgoing_msg.cc | 5 +++-- src/outgoing_msg.h | 7 ++++--- src/socket.cc | 20 ++++++++++---------- src/socket.h | 9 +++++---- src/util/arguments.h | 10 +++++----- 7 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/observer.cc b/src/observer.cc index 95b5b300..1c4b6f93 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -370,14 +370,14 @@ void Observer::Initialize(Module& module, Napi::Object& exports) { void Observer::Poller::ReadableCallback() { assert(read_deferred); - AsyncScope const 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 54dbd7b4..2e0be4ff 100644 --- a/src/observer.h +++ b/src/observer.h @@ -2,6 +2,7 @@ #include +#include #include #include "./closable.h" @@ -38,11 +39,11 @@ class Observer : public Napi::ObjectWrap, public Closable { 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(); @@ -51,7 +52,7 @@ class Observer : public Napi::ObjectWrap, public Closable { } [[nodiscard]] bool ValidateReadable() const { - return socket.HasEvents(); + return socket.get().HasEvents(); } [[nodiscard]] static bool ValidateWritable() { diff --git a/src/outgoing_msg.cc b/src/outgoing_msg.cc index 87ca0904..75bf7355 100644 --- a/src/outgoing_msg.cc +++ b/src/outgoing_msg.cc @@ -1,11 +1,12 @@ #include "./outgoing_msg.h" +#include #include "./module.h" #include "util/error.h" namespace zmq { -OutgoingMsg::OutgoingMsg(Napi::Value value, Module& module) { +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) { @@ -94,7 +95,7 @@ OutgoingMsg::~OutgoingMsg() { } void OutgoingMsg::Reference::Recycle() { - module.MsgTrash.Add(this); + module.get().MsgTrash.Add(this); } OutgoingMsg::Parts::Parts(Napi::Value value, Module& module) { diff --git a/src/outgoing_msg.h b/src/outgoing_msg.h index e0f3170f..9046e9cb 100644 --- a/src/outgoing_msg.h +++ b/src/outgoing_msg.h @@ -3,6 +3,7 @@ #include #include +#include #include "./zmq_inc.h" @@ -23,7 +24,7 @@ class OutgoingMsg { /* 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(); zmq_msg_t* get() { @@ -33,10 +34,10 @@ class OutgoingMsg { private: class Reference { Napi::Reference persistent; - Module& module; + std::reference_wrapper module; public: - explicit Reference(Napi::Value val, Module& module) + explicit Reference(Napi::Value val, std::reference_wrapper module) : persistent(Napi::Persistent(val)), module(module) {} void Recycle(); diff --git a/src/socket.cc b/src/socket.cc index c2e1714e..cdeafc72 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -311,7 +311,7 @@ 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.get(), socket, ZMQ_DONTWAIT) < 0) { @@ -321,7 +321,7 @@ void Socket::Receive(const Napi::Promise::Deferred& res) { } } - list[i++] = part.IntoBuffer(Env()); + list[i_part++] = part.IntoBuffer(Env()); #ifdef ZMQ_HAS_POLLABLE_THREAD_SAFE switch (type) { @@ -953,25 +953,25 @@ 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 const 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 const 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(); } @@ -979,7 +979,7 @@ Napi::Value Socket::Poller::WritePromise(OutgoingMsg::Parts&& parts) { assert(!write_deferred); write_value = std::move(parts); - write_deferred = Napi::Promise::Deferred(socket.Env()); + 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 04321511..601cb3f2 100644 --- a/src/socket.h +++ b/src/socket.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "./closable.h" @@ -70,13 +71,13 @@ 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); @@ -90,11 +91,11 @@ class Socket : public Napi::ObjectWrap, public Closable { } [[nodiscard]] bool ValidateReadable() const { - return socket.HasEvents(ZMQ_POLLIN); + return socket.get().HasEvents(ZMQ_POLLIN); } [[nodiscard]] bool ValidateWritable() const { - return socket.HasEvents(ZMQ_POLLOUT); + return socket.get().HasEvents(ZMQ_POLLOUT); } void ReadableCallback(); diff --git a/src/util/arguments.h b/src/util/arguments.h index e34359f7..e06284e8 100644 --- a/src/util/arguments.h +++ b/src/util/arguments.h @@ -61,7 +61,7 @@ using Optional = Verify; template class Validator { - static constexpr size_t N = sizeof...(F); + static constexpr size_t NumArgs = sizeof...(F); std::tuple validators; public: @@ -85,10 +85,10 @@ class Validator { private: template [[nodiscard]] std::optional eval(const Napi::CallbackInfo& info) const { - if constexpr (I == N) { - if (info.Length() > N) { - auto msg = - "Expected " + std::to_string(N) + " argument" + (N != 1 ? "s" : ""); + 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); } From b8df7ec5dcb1cca82f21099ac683358d057cf425 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 05:58:34 -0700 Subject: [PATCH 22/27] fix: use std::array instead of C-arrays --- src/module.cc | 15 ++++++++++----- src/poller.h | 4 ++-- src/socket.cc | 11 +++++++---- src/util/arguments.h | 2 +- src/util/electron_helper.h | 3 ++- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/module.cc b/src/module.cc index 68491252..1c854f53 100644 --- a/src/module.cc +++ b/src/module.cc @@ -1,4 +1,6 @@ +#include + #include "./module.h" #include "./context.h" @@ -58,16 +60,19 @@ Napi::Object Capabilities(const Napi::Env& env) { } Napi::Value CurveKeyPair(const Napi::CallbackInfo& info) { - char public_key[41]; - char secret_key[41]; - if (zmq_curve_keypair(public_key, secret_key) < 0) { + 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; } diff --git a/src/poller.h b/src/poller.h index e1b1391e..09a3aac4 100644 --- a/src/poller.h +++ b/src/poller.h @@ -24,11 +24,11 @@ class Poller { ZMQ style edge-triggered, with READABLE state indicating that ANY event may be present on the corresponding ZMQ socket. */ int32_t Initialize( - Napi::Env env, uv_os_sock_t& fd, std::function finalizer = nullptr) { + 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.get(), fd); err != 0) { + if (auto err = uv_poll_init_socket(loop, poll.get(), file_descriptor); err != 0) { return err; } diff --git a/src/socket.cc b/src/socket.cc index cdeafc72..f8f60312 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -4,6 +4,7 @@ #include #include #include +#include #include "./context.h" #include "./incoming_msg.h" @@ -778,9 +779,10 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { 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(); } @@ -788,8 +790,9 @@ Napi::Value Socket::GetSockOpt(const Napi::CallbackInfo& info) { if (length == 0 || (length == 1 && value[0] == 0)) { return Env().Null(); } + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) value[length] = '\0'; - return Napi::String::New(Env(), value); + return Napi::String::New(Env(), value.data()); } template <> diff --git a/src/util/arguments.h b/src/util/arguments.h index e06284e8..bbda1cca 100644 --- a/src/util/arguments.h +++ b/src/util/arguments.h @@ -28,7 +28,7 @@ struct Not { template class Verify { - const std::string_view msg; + /* const */ std::string_view msg; public: constexpr explicit Verify(std::string_view msg) noexcept : msg(msg) {} diff --git a/src/util/electron_helper.h b/src/util/electron_helper.h index 2520ab2f..c9a41c4e 100644 --- a/src/util/electron_helper.h +++ b/src/util/electron_helper.h @@ -36,7 +36,8 @@ inline bool hasElectronMemoryCage(const Napi::Env& env) { .ToString() .Utf8Value(); int const majorVer = stoi(first_component(electronVers)); - if (majorVer >= 21) { + static constexpr auto electron_memory_cage_version = 21; + if (majorVer >= electron_memory_cage_version) { hasElectronMemoryCageCache = true; } } From 9b4dd7e7e0eeb4e35d0775df6415ade713a839af Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 06:08:25 -0700 Subject: [PATCH 23/27] fix: fix buffer to value safe conversion --- src/incoming_msg.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/incoming_msg.cc b/src/incoming_msg.cc index 7ae55d62..0d105405 100644 --- a/src/incoming_msg.cc +++ b/src/incoming_msg.cc @@ -42,21 +42,22 @@ Napi::Value IncomingMsg::IntoBuffer(const Napi::Env& env) { Napi::MemoryManagement::AdjustExternalMemory( env, static_cast(length)); - auto release = [](const Napi::Env& env, uint8_t*, Reference* 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() { From 3b14017512b7e56936de7b2fe5081d6bc315d201 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 06:12:16 -0700 Subject: [PATCH 24/27] fix: use a lambda instead of goto --- src/observer.cc | 21 ++++++++++----------- src/socket.cc | 31 +++++++++++++++---------------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/observer.cc b/src/observer.cc index 1c4b6f93..1626937a 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -162,31 +162,30 @@ Observer::Observer(const Napi::CallbackInfo& info) 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, &file_descriptor, &length) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); - goto error; + error(); } 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: - [[maybe_unused]] auto err = zmq_close(socket); - assert(err == 0); - - socket = nullptr; } Observer::~Observer() { diff --git a/src/socket.cc b/src/socket.cc index f8f60312..c740a14d 100644 --- a/src/socket.cc +++ b/src/socket.cc @@ -1,10 +1,10 @@ #include "./socket.h" +#include #include #include #include #include -#include #include "./context.h" #include "./incoming_msg.h" @@ -105,13 +105,20 @@ Socket::Socket(const Napi::CallbackInfo& info) 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 { int value = 0; 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 != 0); @@ -126,7 +133,7 @@ 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 @@ -139,31 +146,31 @@ Socket::Socket(const Napi::CallbackInfo& info) 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(file_descriptor); if (zmq_getsockopt(socket, ZMQ_FD, &file_descriptor, &length) < 0) { ErrnoException(Env(), zmq_errno()).ThrowAsJavaScriptException(); - goto error; + error(); } } if (poller.Initialize(Env(), file_descriptor, finalize) < 0) { ErrnoException(Env(), errno).ThrowAsJavaScriptException(); - goto error; + error(); } /* Initialization was successful, register the socket for cleanup. */ @@ -177,14 +184,6 @@ Socket::Socket(const Napi::CallbackInfo& info) if (info[1].IsObject()) { Assign(info.This().As(), info[1].As()); } - - return; - -error: - [[maybe_unused]] auto err = zmq_close(socket); - assert(err == 0); - - socket = nullptr; } Socket::~Socket() { From 118aa40c825d2f9439549de158a3cbc1faa0719e Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 06:18:32 -0700 Subject: [PATCH 25/27] fix: silence poller UV reinterpret cast warnings --- src/module.h | 6 +++--- src/poller.h | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/module.h b/src/module.h index 16ecc77a..5e3a70a7 100644 --- a/src/module.h +++ b/src/module.h @@ -18,10 +18,10 @@ struct Terminator { assert(context != nullptr); #ifdef ZMQ_BLOCKY - bool const blocky = zmq_ctx_get(context, ZMQ_BLOCKY) != 0; + 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 @@ -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(), diff --git a/src/poller.h b/src/poller.h index 09a3aac4..cbeb8088 100644 --- a/src/poller.h +++ b/src/poller.h @@ -74,6 +74,7 @@ class Poller { [[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); }, @@ -98,6 +99,7 @@ class Poller { [[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); }, @@ -164,6 +166,7 @@ class Poller { There is no guarantee that any events are available. */ 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(); From a91d6f728fbd7da9a3d56c71bdaed16bd65848eb Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 06:26:41 -0700 Subject: [PATCH 26/27] fix: remove unnecessary macros --- src/observer.cc | 1 + src/util/error.h | 258 ++++++++++++++++++++++++++++++----------------- 2 files changed, 169 insertions(+), 90 deletions(-) diff --git a/src/observer.cc b/src/observer.cc index 1626937a..d615f157 100644 --- a/src/observer.cc +++ b/src/observer.cc @@ -95,6 +95,7 @@ constexpr const char* AuthError(uint32_t val) { #ifdef ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL 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); diff --git a/src/util/error.h b/src/util/error.h index c63db269..5930ef3f 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -8,8 +8,8 @@ #include "../zmq_inc.h" namespace zmq { -static constexpr const char* ErrnoMessage(int32_t errorno); -static 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. */ inline void Warn(const Napi::Env& env, const std::string& msg) { @@ -54,7 +54,7 @@ inline Napi::Error ErrnoException( } /* Convert errno to human readable error message. */ -static 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: @@ -79,346 +79,424 @@ static 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 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: From e9086988b1cce9a2ae949a019cad584c43839385 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 23 Oct 2024 06:37:32 -0700 Subject: [PATCH 27/27] build: disable shadow warnings on AppleClang --- CMakeLists.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fe2dce4b..90cfaaf0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -102,12 +102,13 @@ project_options(ENABLE_CACHE ENABLE_COMPILE_COMMANDS_SYMLINK) file(GLOB_RECURSE SOURCES "./src/*.cc") add_library(addon SHARED ${SOURCES}) -if(CMAKE_CXX_COMPILER_ID STREQUAL GNU) +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) endif()