From 0c6455278da681d20fa8297f4a1c301cb78506f5 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Sun, 24 Jul 2016 05:13:19 +0200 Subject: [PATCH] src: make ReqWrap req_ member private This commit attempts to address one of the items in https://github.com/nodejs/node/issues/4641 which is related to src/req-wrap.h and making the req_ member private. PR-URL: https://github.com/nodejs/node/pull/8532 Reviewed-By: Ben Noordhuis Reviewed-By: Anna Henningsen --- src/cares_wrap.cc | 4 ++-- src/node_file.cc | 18 +++++++++--------- src/pipe_wrap.cc | 2 +- src/req-wrap.h | 10 ++++++++-- src/stream_base.h | 9 +++++++++ src/stream_wrap.cc | 12 +++++++----- src/tcp_wrap.cc | 4 ++-- src/udp_wrap.cc | 2 +- 8 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 8ceec9d5af..44048c5dc7 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1146,7 +1146,7 @@ static void GetAddrInfo(const FunctionCallbackInfo& args) { hints.ai_flags = flags; int err = uv_getaddrinfo(env->event_loop(), - &req_wrap->req_, + req_wrap->req(), AfterGetAddrInfo, *hostname, nullptr, @@ -1176,7 +1176,7 @@ static void GetNameInfo(const FunctionCallbackInfo& args) { GetNameInfoReqWrap* req_wrap = new GetNameInfoReqWrap(env, req_wrap_obj); int err = uv_getnameinfo(env->event_loop(), - &req_wrap->req_, + req_wrap->req(), AfterGetNameInfo, (struct sockaddr*)&addr, NI_NAMEREQD); diff --git a/src/node_file.cc b/src/node_file.cc index 4238a69664..70f99c09f6 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -137,7 +137,7 @@ static inline bool IsInt64(double x) { static void After(uv_fs_t *req) { FSReqWrap* req_wrap = static_cast(req->data); - CHECK_EQ(&req_wrap->req_, req); + CHECK_EQ(req_wrap->req(), req); req_wrap->ReleaseEarly(); // Free memory that's no longer used now. Environment* env = req_wrap->env(); @@ -320,7 +320,7 @@ static void After(uv_fs_t *req) { req_wrap->MakeCallback(env->oncomplete_string(), argc, argv); - uv_fs_req_cleanup(&req_wrap->req_); + uv_fs_req_cleanup(req_wrap->req()); req_wrap->Dispose(); } @@ -337,18 +337,18 @@ class fs_req_wrap { }; -#define ASYNC_DEST_CALL(func, req, dest, encoding, ...) \ +#define ASYNC_DEST_CALL(func, request, dest, encoding, ...) \ Environment* env = Environment::GetCurrent(args); \ - CHECK(req->IsObject()); \ - FSReqWrap* req_wrap = FSReqWrap::New(env, req.As(), \ + CHECK(request->IsObject()); \ + FSReqWrap* req_wrap = FSReqWrap::New(env, request.As(), \ #func, dest, encoding); \ int err = uv_fs_ ## func(env->event_loop(), \ - &req_wrap->req_, \ + req_wrap->req(), \ __VA_ARGS__, \ After); \ req_wrap->Dispatched(); \ if (err < 0) { \ - uv_fs_t* uv_req = &req_wrap->req_; \ + uv_fs_t* uv_req = req_wrap->req(); \ uv_req->result = err; \ uv_req->path = nullptr; \ After(uv_req); \ @@ -1164,7 +1164,7 @@ static void WriteString(const FunctionCallbackInfo& args) { FSReqWrap* req_wrap = FSReqWrap::New(env, req.As(), "write", buf, UTF8, ownership); int err = uv_fs_write(env->event_loop(), - &req_wrap->req_, + req_wrap->req(), fd, &uvbuf, 1, @@ -1172,7 +1172,7 @@ static void WriteString(const FunctionCallbackInfo& args) { After); req_wrap->Dispatched(); if (err < 0) { - uv_fs_t* uv_req = &req_wrap->req_; + uv_fs_t* uv_req = req_wrap->req(); uv_req->result = err; uv_req->path = nullptr; After(uv_req); diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 77476d52db..8fd2f8f5f3 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -168,7 +168,7 @@ void PipeWrap::Connect(const FunctionCallbackInfo& args) { ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_PIPECONNECTWRAP); - uv_pipe_connect(&req_wrap->req_, + uv_pipe_connect(req_wrap->req(), &wrap->handle_, *name, AfterConnect); diff --git a/src/req-wrap.h b/src/req-wrap.h index e226dc6395..0fddae6746 100644 --- a/src/req-wrap.h +++ b/src/req-wrap.h @@ -18,13 +18,19 @@ class ReqWrap : public AsyncWrap { AsyncWrap::ProviderType provider); inline ~ReqWrap() override; inline void Dispatched(); // Call this after the req has been dispatched. + T* req() { return &req_; } private: friend class Environment; ListNode req_wrap_queue_; - public: - T req_; // Must be last. TODO(bnoordhuis) Make private. + protected: + // req_wrap_queue_ needs to be at a fixed offset from the start of the class + // because it is used by ContainerOf to calculate the address of the embedding + // ReqWrap. ContainerOf compiles down to simple, fixed pointer arithmetic. + // sizeof(req_) depends on the type of T, so req_wrap_queue_ would + // no longer be at a fixed offset if it came after req_. + T req_; }; } // namespace node diff --git a/src/stream_base.h b/src/stream_base.h index f3ad51ded2..faddee88c8 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -8,6 +8,7 @@ #include "req-wrap.h" #include "req-wrap-inl.h" #include "node.h" +#include "util.h" #include "v8.h" @@ -56,6 +57,10 @@ class ShutdownWrap : public ReqWrap, CHECK(args.IsConstructCall()); } + static ShutdownWrap* from_req(uv_shutdown_t* req) { + return ContainerOf(&ShutdownWrap::req_, req); + } + inline StreamBase* wrap() const { return wrap_; } size_t self_size() const override { return sizeof(*this); } @@ -82,6 +87,10 @@ class WriteWrap: public ReqWrap, CHECK(args.IsConstructCall()); } + static WriteWrap* from_req(uv_write_t* req) { + return ContainerOf(&WriteWrap::req_, req); + } + static const size_t kAlignSize = 16; protected: diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 2b50895c9f..d294b641ff 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -276,14 +276,15 @@ void StreamWrap::SetBlocking(const FunctionCallbackInfo& args) { int StreamWrap::DoShutdown(ShutdownWrap* req_wrap) { int err; - err = uv_shutdown(&req_wrap->req_, stream(), AfterShutdown); + err = uv_shutdown(req_wrap->req(), stream(), AfterShutdown); req_wrap->Dispatched(); return err; } void StreamWrap::AfterShutdown(uv_shutdown_t* req, int status) { - ShutdownWrap* req_wrap = ContainerOf(&ShutdownWrap::req_, req); + ShutdownWrap* req_wrap = ShutdownWrap::from_req(req); + CHECK_NE(req_wrap, nullptr); HandleScope scope(req_wrap->env()->isolate()); Context::Scope context_scope(req_wrap->env()->context()); req_wrap->Done(status); @@ -336,9 +337,9 @@ int StreamWrap::DoWrite(WriteWrap* w, uv_stream_t* send_handle) { int r; if (send_handle == nullptr) { - r = uv_write(&w->req_, stream(), bufs, count, AfterWrite); + r = uv_write(w->req(), stream(), bufs, count, AfterWrite); } else { - r = uv_write2(&w->req_, stream(), bufs, count, send_handle, AfterWrite); + r = uv_write2(w->req(), stream(), bufs, count, send_handle, AfterWrite); } if (!r) { @@ -360,7 +361,8 @@ int StreamWrap::DoWrite(WriteWrap* w, void StreamWrap::AfterWrite(uv_write_t* req, int status) { - WriteWrap* req_wrap = ContainerOf(&WriteWrap::req_, req); + WriteWrap* req_wrap = WriteWrap::from_req(req); + CHECK_NE(req_wrap, nullptr); HandleScope scope(req_wrap->env()->isolate()); Context::Scope context_scope(req_wrap->env()->context()); req_wrap->Done(status); diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index f2e972970b..b2617a5695 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -257,7 +257,7 @@ void TCPWrap::Connect(const FunctionCallbackInfo& args) { if (err == 0) { ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); - err = uv_tcp_connect(&req_wrap->req_, + err = uv_tcp_connect(req_wrap->req(), &wrap->handle_, reinterpret_cast(&addr), AfterConnect); @@ -292,7 +292,7 @@ void TCPWrap::Connect6(const FunctionCallbackInfo& args) { if (err == 0) { ConnectWrap* req_wrap = new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP); - err = uv_tcp_connect(&req_wrap->req_, + err = uv_tcp_connect(req_wrap->req(), &wrap->handle_, reinterpret_cast(&addr), AfterConnect); diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 7fba279fca..c8009a5276 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -305,7 +305,7 @@ void UDPWrap::DoSend(const FunctionCallbackInfo& args, int family) { } if (err == 0) { - err = uv_udp_send(&req_wrap->req_, + err = uv_udp_send(req_wrap->req(), &wrap->handle_, *bufs, count,