Skip to content

Commit

Permalink
src: unifying PipeConnectWrap and TCPConnectWrap
Browse files Browse the repository at this point in the history
This commit attempts to address one of the items in nodejs#4641 which is
related to src/pipe_wrap.cc and src/tcp_wrap.cc. Currently both
pipe_wrap.cc and tcp_wrap.cc contain a class that are almost
identical. This commit extracts these parts into a separate class
that both can share.

PR-URL: nodejs#7501
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
danbev authored and addaleax committed Jul 28, 2016
1 parent c948877 commit b896057
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 51 deletions.
2 changes: 2 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@
'src/fs_event_wrap.cc',
'src/cares_wrap.cc',
'src/connection_wrap.cc',
'src/connect_wrap.cc',
'src/handle_wrap.cc',
'src/js_stream.cc',
'src/node.cc',
Expand Down Expand Up @@ -179,6 +180,7 @@
'src/base-object.h',
'src/base-object-inl.h',
'src/connection_wrap.h',
'src/connect_wrap.h',
'src/debug-agent.h',
'src/env.h',
'src/env-inl.h',
Expand Down
22 changes: 22 additions & 0 deletions src/connect_wrap.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include "connect_wrap.h"

#include "env.h"
#include "env-inl.h"
#include "req-wrap.h"
#include "req-wrap-inl.h"
#include "util.h"
#include "util-inl.h"

namespace node {

using v8::Local;
using v8::Object;


ConnectWrap::ConnectWrap(Environment* env,
Local<Object> req_wrap_obj,
AsyncWrap::ProviderType provider) : ReqWrap(env, req_wrap_obj, provider) {
Wrap(req_wrap_obj, this);
}

} // namespace node
26 changes: 26 additions & 0 deletions src/connect_wrap.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#ifndef SRC_CONNECT_WRAP_H_
#define SRC_CONNECT_WRAP_H_

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include "env.h"
#include "req-wrap.h"
#include "async-wrap.h"
#include "v8.h"

namespace node {

class ConnectWrap : public ReqWrap<uv_connect_t> {
public:
ConnectWrap(Environment* env,
v8::Local<v8::Object> req_wrap_obj,
AsyncWrap::ProviderType provider);

size_t self_size() const override { return sizeof(*this); }
};

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#endif // SRC_CONNECT_WRAP_H_
34 changes: 8 additions & 26 deletions src/pipe_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
#include "node.h"
#include "node_buffer.h"
#include "node_wrap.h"
#include "req-wrap.h"
#include "req-wrap-inl.h"
#include "connect_wrap.h"
#include "stream_wrap.h"
#include "util-inl.h"
#include "util.h"
Expand All @@ -31,26 +30,6 @@ using v8::String;
using v8::Value;


// TODO(bnoordhuis) share with TCPWrap?
class PipeConnectWrap : public ReqWrap<uv_connect_t> {
public:
PipeConnectWrap(Environment* env, Local<Object> req_wrap_obj);

size_t self_size() const override { return sizeof(*this); }
};


PipeConnectWrap::PipeConnectWrap(Environment* env, Local<Object> req_wrap_obj)
: ReqWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_PIPECONNECTWRAP) {
Wrap(req_wrap_obj, this);
}


static void NewPipeConnectWrap(const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
}


Local<Object> PipeWrap::Instantiate(Environment* env, AsyncWrap* parent) {
EscapableHandleScope handle_scope(env->isolate());
CHECK_EQ(false, env->pipe_constructor_template().IsEmpty());
Expand Down Expand Up @@ -92,8 +71,10 @@ void PipeWrap::Initialize(Local<Object> target,
env->set_pipe_constructor_template(t);

// Create FunctionTemplate for PipeConnectWrap.
Local<FunctionTemplate> cwt =
FunctionTemplate::New(env->isolate(), NewPipeConnectWrap);
auto constructor = [](const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
};
auto cwt = FunctionTemplate::New(env->isolate(), constructor);
cwt->InstanceTemplate()->SetInternalFieldCount(1);
cwt->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "PipeConnectWrap"));
target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "PipeConnectWrap"),
Expand Down Expand Up @@ -163,7 +144,7 @@ void PipeWrap::Listen(const FunctionCallbackInfo<Value>& args) {

// TODO(bnoordhuis) Maybe share this with TCPWrap?
void PipeWrap::AfterConnect(uv_connect_t* req, int status) {
PipeConnectWrap* req_wrap = static_cast<PipeConnectWrap*>(req->data);
ConnectWrap* req_wrap = static_cast<ConnectWrap*>(req->data);
PipeWrap* wrap = static_cast<PipeWrap*>(req->handle->data);
CHECK_EQ(req_wrap->env(), wrap->env());
Environment* env = wrap->env();
Expand Down Expand Up @@ -226,7 +207,8 @@ void PipeWrap::Connect(const FunctionCallbackInfo<Value>& args) {
Local<Object> req_wrap_obj = args[0].As<Object>();
node::Utf8Value name(env->isolate(), args[1]);

PipeConnectWrap* req_wrap = new PipeConnectWrap(env, req_wrap_obj);
ConnectWrap* req_wrap =
new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_PIPECONNECTWRAP);
uv_pipe_connect(&req_wrap->req_,
&wrap->handle_,
*name,
Expand Down
35 changes: 10 additions & 25 deletions src/tcp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
#include "handle_wrap.h"
#include "node_buffer.h"
#include "node_wrap.h"
#include "req-wrap.h"
#include "req-wrap-inl.h"
#include "connect_wrap.h"
#include "stream_wrap.h"
#include "util.h"
#include "util-inl.h"
Expand All @@ -32,24 +31,6 @@ using v8::String;
using v8::Value;


class TCPConnectWrap : public ReqWrap<uv_connect_t> {
public:
TCPConnectWrap(Environment* env, Local<Object> req_wrap_obj);
size_t self_size() const override { return sizeof(*this); }
};


TCPConnectWrap::TCPConnectWrap(Environment* env, Local<Object> req_wrap_obj)
: ReqWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP) {
Wrap(req_wrap_obj, this);
}


static void NewTCPConnectWrap(const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
}


Local<Object> TCPWrap::Instantiate(Environment* env, AsyncWrap* parent) {
EscapableHandleScope handle_scope(env->isolate());
CHECK_EQ(env->tcp_constructor_template().IsEmpty(), false);
Expand Down Expand Up @@ -112,8 +93,10 @@ void TCPWrap::Initialize(Local<Object> target,
env->set_tcp_constructor_template(t);

// Create FunctionTemplate for TCPConnectWrap.
Local<FunctionTemplate> cwt =
FunctionTemplate::New(env->isolate(), NewTCPConnectWrap);
auto constructor = [](const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
};
auto cwt = FunctionTemplate::New(env->isolate(), constructor);
cwt->InstanceTemplate()->SetInternalFieldCount(1);
cwt->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "TCPConnectWrap"));
target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "TCPConnectWrap"),
Expand Down Expand Up @@ -253,7 +236,7 @@ void TCPWrap::Listen(const FunctionCallbackInfo<Value>& args) {


void TCPWrap::AfterConnect(uv_connect_t* req, int status) {
TCPConnectWrap* req_wrap = static_cast<TCPConnectWrap*>(req->data);
ConnectWrap* req_wrap = static_cast<ConnectWrap*>(req->data);
TCPWrap* wrap = static_cast<TCPWrap*>(req->handle->data);
CHECK_EQ(req_wrap->env(), wrap->env());
Environment* env = wrap->env();
Expand Down Expand Up @@ -300,7 +283,8 @@ void TCPWrap::Connect(const FunctionCallbackInfo<Value>& args) {
int err = uv_ip4_addr(*ip_address, port, &addr);

if (err == 0) {
TCPConnectWrap* req_wrap = new TCPConnectWrap(env, req_wrap_obj);
ConnectWrap* req_wrap =
new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP);
err = uv_tcp_connect(&req_wrap->req_,
&wrap->handle_,
reinterpret_cast<const sockaddr*>(&addr),
Expand Down Expand Up @@ -334,7 +318,8 @@ void TCPWrap::Connect6(const FunctionCallbackInfo<Value>& args) {
int err = uv_ip6_addr(*ip_address, port, &addr);

if (err == 0) {
TCPConnectWrap* req_wrap = new TCPConnectWrap(env, req_wrap_obj);
ConnectWrap* req_wrap =
new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP);
err = uv_tcp_connect(&req_wrap->req_,
&wrap->handle_,
reinterpret_cast<const sockaddr*>(&addr),
Expand Down

0 comments on commit b896057

Please sign in to comment.