From 4f84dde2c60c8db850ee941877cabc250273524c Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 22 Oct 2018 08:48:40 +0200 Subject: [PATCH 1/6] src: use smart pointers in cares_wrap.cc --- src/cares_wrap.cc | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 1a83a1cf7adf09..973de33472e0d2 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1797,14 +1797,15 @@ static void Query(const FunctionCallbackInfo& args) { Local req_wrap_obj = args[0].As(); Local string = args[1].As(); - Wrap* wrap = new Wrap(channel, req_wrap_obj); + std::unique_ptr wrap {new Wrap(channel, req_wrap_obj)}; node::Utf8Value name(env->isolate(), string); channel->ModifyActivityQueryCount(1); int err = wrap->Send(*name); if (err) { channel->ModifyActivityQueryCount(-1); - delete wrap; + } else { + wrap.release(); } args.GetReturnValue().Set(err); @@ -1812,7 +1813,8 @@ static void Query(const FunctionCallbackInfo& args) { void AfterGetAddrInfo(uv_getaddrinfo_t* req, int status, struct addrinfo* res) { - GetAddrInfoReqWrap* req_wrap = static_cast(req->data); + std::unique_ptr req_wrap { + static_cast(req->data)}; Environment* env = req_wrap->env(); HandleScope handle_scope(env->isolate()); @@ -1869,13 +1871,11 @@ void AfterGetAddrInfo(uv_getaddrinfo_t* req, int status, struct addrinfo* res) { uv_freeaddrinfo(res); TRACE_EVENT_NESTABLE_ASYNC_END2( - TRACING_CATEGORY_NODE2(dns, native), "lookup", req_wrap, + TRACING_CATEGORY_NODE2(dns, native), "lookup", req_wrap.get(), "count", n, "verbatim", verbatim); // Make the callback into JavaScript req_wrap->MakeCallback(env->oncomplete_string(), arraysize(argv), argv); - - delete req_wrap; } @@ -1883,7 +1883,8 @@ void AfterGetNameInfo(uv_getnameinfo_t* req, int status, const char* hostname, const char* service) { - GetNameInfoReqWrap* req_wrap = static_cast(req->data); + std::unique_ptr req_wrap { + static_cast(req->data)}; Environment* env = req_wrap->env(); HandleScope handle_scope(env->isolate()); @@ -1904,14 +1905,12 @@ void AfterGetNameInfo(uv_getnameinfo_t* req, } TRACE_EVENT_NESTABLE_ASYNC_END2( - TRACING_CATEGORY_NODE2(dns, native), "lookupService", req_wrap, + TRACING_CATEGORY_NODE2(dns, native), "lookupService", req_wrap.get(), "hostname", TRACE_STR_COPY(hostname), "service", TRACE_STR_COPY(service)); // Make the callback into JavaScript req_wrap->MakeCallback(env->oncomplete_string(), arraysize(argv), argv); - - delete req_wrap; } using ParseIPResult = decltype(static_cast(0)->addr); @@ -1971,7 +1970,8 @@ void GetAddrInfo(const FunctionCallbackInfo& args) { CHECK(0 && "bad address family"); } - auto req_wrap = new GetAddrInfoReqWrap(env, req_wrap_obj, args[4]->IsTrue()); + std::unique_ptr req_wrap { + new GetAddrInfoReqWrap(env, req_wrap_obj, args[4]->IsTrue())}; struct addrinfo hints; memset(&hints, 0, sizeof(struct addrinfo)); @@ -1980,7 +1980,7 @@ void GetAddrInfo(const FunctionCallbackInfo& args) { hints.ai_flags = flags; TRACE_EVENT_NESTABLE_ASYNC_BEGIN2( - TRACING_CATEGORY_NODE2(dns, native), "lookup", req_wrap, + TRACING_CATEGORY_NODE2(dns, native), "lookup", req_wrap.get(), "hostname", TRACE_STR_COPY(*hostname), "family", family == AF_INET ? "ipv4" : family == AF_INET6 ? "ipv6" : "unspec"); @@ -1990,8 +1990,8 @@ void GetAddrInfo(const FunctionCallbackInfo& args) { *hostname, nullptr, &hints); - if (err) - delete req_wrap; + if (!err) + req_wrap.release(); args.GetReturnValue().Set(err); } @@ -2011,18 +2011,19 @@ void GetNameInfo(const FunctionCallbackInfo& args) { CHECK(uv_ip4_addr(*ip, port, reinterpret_cast(&addr)) == 0 || uv_ip6_addr(*ip, port, reinterpret_cast(&addr)) == 0); - GetNameInfoReqWrap* req_wrap = new GetNameInfoReqWrap(env, req_wrap_obj); + std::unique_ptr req_wrap { + new GetNameInfoReqWrap(env, req_wrap_obj)}; TRACE_EVENT_NESTABLE_ASYNC_BEGIN2( - TRACING_CATEGORY_NODE2(dns, native), "lookupService", req_wrap, + TRACING_CATEGORY_NODE2(dns, native), "lookupService", req_wrap.get(), "ip", TRACE_STR_COPY(*ip), "port", port); int err = req_wrap->Dispatch(uv_getnameinfo, AfterGetNameInfo, reinterpret_cast(&addr), NI_NAMEREQD); - if (err) - delete req_wrap; + if (!err) + req_wrap.release(); args.GetReturnValue().Set(err); } From c6b4ed952bb0e39d3050bc65852dd3c9afe2e0c1 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 23 Oct 2018 05:36:16 +0200 Subject: [PATCH 2/6] squash: fix err checks --- src/cares_wrap.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 973de33472e0d2..48af3b1983f9d6 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1990,7 +1990,7 @@ void GetAddrInfo(const FunctionCallbackInfo& args) { *hostname, nullptr, &hints); - if (!err) + if (err == 0) req_wrap.release(); args.GetReturnValue().Set(err); @@ -2022,7 +2022,7 @@ void GetNameInfo(const FunctionCallbackInfo& args) { AfterGetNameInfo, reinterpret_cast(&addr), NI_NAMEREQD); - if (!err) + if (err == 0) req_wrap.release(); args.GetReturnValue().Set(err); From a6ee1d3f316cff084f1d528fb4572ab1c652cb0e Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 25 Oct 2018 07:42:44 +0200 Subject: [PATCH 3/6] squash: use std::make_unique where applicable --- src/cares_wrap.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 48af3b1983f9d6..43bf26d861a497 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1797,7 +1797,7 @@ static void Query(const FunctionCallbackInfo& args) { Local req_wrap_obj = args[0].As(); Local string = args[1].As(); - std::unique_ptr wrap {new Wrap(channel, req_wrap_obj)}; + auto wrap = std::make_unique(channel, req_wrap_obj); node::Utf8Value name(env->isolate(), string); channel->ModifyActivityQueryCount(1); @@ -1970,8 +1970,9 @@ void GetAddrInfo(const FunctionCallbackInfo& args) { CHECK(0 && "bad address family"); } - std::unique_ptr req_wrap { - new GetAddrInfoReqWrap(env, req_wrap_obj, args[4]->IsTrue())}; + auto req_wrap = std::make_unique(env, + req_wrap_obj, + args[4]->IsTrue()); struct addrinfo hints; memset(&hints, 0, sizeof(struct addrinfo)); @@ -2011,8 +2012,7 @@ void GetNameInfo(const FunctionCallbackInfo& args) { CHECK(uv_ip4_addr(*ip, port, reinterpret_cast(&addr)) == 0 || uv_ip6_addr(*ip, port, reinterpret_cast(&addr)) == 0); - std::unique_ptr req_wrap { - new GetNameInfoReqWrap(env, req_wrap_obj)}; + auto req_wrap = std::make_unique(env, req_wrap_obj); TRACE_EVENT_NESTABLE_ASYNC_BEGIN2( TRACING_CATEGORY_NODE2(dns, native), "lookupService", req_wrap.get(), From bb9f5c755aa493bfecb7580e239391df4d9a240f Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 25 Oct 2018 08:22:41 +0200 Subject: [PATCH 4/6] squash: wrap release in USE macro --- src/cares_wrap.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 43bf26d861a497..c8e2c0ec7cb0dd 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1805,7 +1805,7 @@ static void Query(const FunctionCallbackInfo& args) { if (err) { channel->ModifyActivityQueryCount(-1); } else { - wrap.release(); + USE(wrap.release()); } args.GetReturnValue().Set(err); From a0c36a58d20c8475a74a1bd996e6830fa525b7c9 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 30 Oct 2018 06:04:38 +0100 Subject: [PATCH 5/6] squash: add missing USE wrapping --- src/cares_wrap.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index c8e2c0ec7cb0dd..a03c293445a5a7 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1992,7 +1992,7 @@ void GetAddrInfo(const FunctionCallbackInfo& args) { nullptr, &hints); if (err == 0) - req_wrap.release(); + USE(req_wrap.release()); args.GetReturnValue().Set(err); } @@ -2023,7 +2023,7 @@ void GetNameInfo(const FunctionCallbackInfo& args) { reinterpret_cast(&addr), NI_NAMEREQD); if (err == 0) - req_wrap.release(); + USE(req_wrap.release()); args.GetReturnValue().Set(err); } From 25c405e97870acedebf68a53e3f220227d831794 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 30 Oct 2018 06:22:51 +0100 Subject: [PATCH 6/6] squash: add comment about release() --- src/cares_wrap.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index a03c293445a5a7..de4397f228e0c4 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1805,6 +1805,7 @@ static void Query(const FunctionCallbackInfo& args) { if (err) { channel->ModifyActivityQueryCount(-1); } else { + // Release ownership of the pointer allowing the ownership to be transferred USE(wrap.release()); } @@ -1992,6 +1993,7 @@ void GetAddrInfo(const FunctionCallbackInfo& args) { nullptr, &hints); if (err == 0) + // Release ownership of the pointer allowing the ownership to be transferred USE(req_wrap.release()); args.GetReturnValue().Set(err); @@ -2023,6 +2025,7 @@ void GetNameInfo(const FunctionCallbackInfo& args) { reinterpret_cast(&addr), NI_NAMEREQD); if (err == 0) + // Release ownership of the pointer allowing the ownership to be transferred USE(req_wrap.release()); args.GetReturnValue().Set(err);