Skip to content

Commit 34d5abc

Browse files
danbevdanbev
committed
extracting UVHandle() and handle_ to ConnectionWrap.
I originally just wanted to move the handle_ member from tcp_wrap and pipe_wrap without changing its name. But I ran into an issue which I believe is because tcp_wrap and pipe_wrap now extend ConnectionWrap. Both classes are derived from StreamWrap which has an inheritance tree that looks like this: class PipeWrap : public StreamWrap, ConnectionWrap<PipeWrap, uv_pipe_t> class StreamWrap : public HandleWrap, public StreamBase class HandleWrap : public AsyncWrap class AsyncWrap : public BaseObject BaseObject has the following private member: v8::Persistent<v8::Object> handle_; The compiler complains that 'handle_' is found in multiple base classes of different types: ../src/pipe_wrap.cc:130:50: error: member 'handle_' found in multiple base classes of different types reinterpret_cast<uv_stream_t*>(&handle_), ^ ../src/base-object.h:47:30: note: member found by ambiguous name lookup v8::Persistent<v8::Object> handle_; It turns out that name lookup is performed before access rules are considered. C++ standard section 3.4: "The access rules (Clause 11) are considered only once name lookup and function overload resolution (if applicable) have succeeded." It is possible to be explicit about the type you want using the resolution operator ::. So we could use ConnectionWrap::handle_ to tell the compiler which type we want. But going down this route still caused changes to a number of files and I think the code is somewhat clearer using a different name for the handle, and there is no confusion with the handle_ member in BaseObject. Being fairly new to C++ and to the project there this is probably a gap in my knowledge about the best way to solve this. Looking forward to hear about other options/ideas for this.
1 parent a3b976e commit 34d5abc

File tree

9 files changed

+48
-52
lines changed

9 files changed

+48
-52
lines changed

‎src/connection_wrap.cc‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ template<typename WrapType, typename UVType>
2121
void ConnectionWrap<WrapType, UVType>::OnConnection(uv_stream_t* handle,
2222
int status) {
2323
WrapType* wrap_data = static_cast<WrapType*>(handle->data);
24-
CHECK_EQ(&wrap_data->handle_, reinterpret_cast<UVType*>(handle));
24+
CHECK_EQ(&wrap_data->uvhandle_, reinterpret_cast<UVType*>(handle));
2525

2626
Environment* env = wrap_data->env();
2727
HandleScope handle_scope(env->isolate());
@@ -43,7 +43,8 @@ void ConnectionWrap<WrapType, UVType>::OnConnection(uv_stream_t* handle,
4343
// Unwrap the client javascript object.
4444
WrapType* wrap;
4545
ASSIGN_OR_RETURN_UNWRAP(&wrap, client_obj);
46-
uv_stream_t* client_handle = reinterpret_cast<uv_stream_t*>(&wrap->handle_);
46+
uv_stream_t* client_handle =
47+
reinterpret_cast<uv_stream_t*>(&wrap->uvhandle_);
4748
if (uv_accept(handle, client_handle))
4849
return;
4950

@@ -59,5 +60,14 @@ template void ConnectionWrap<PipeWrap, uv_pipe_t>::OnConnection(
5960
template void ConnectionWrap<TCPWrap, uv_tcp_t>::OnConnection(
6061
uv_stream_t* handle, int status);
6162

63+
template<typename WrapType, typename UVType>
64+
UVType* ConnectionWrap<WrapType, UVType>::UVHandle() {
65+
return &uvhandle_;
66+
}
67+
68+
template uv_pipe_t* ConnectionWrap<PipeWrap, uv_pipe_t>::UVHandle();
69+
70+
template uv_tcp_t* ConnectionWrap<TCPWrap, uv_tcp_t>::UVHandle();
71+
6272

6373
} // namespace node

‎src/connection_wrap.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,12 @@ namespace node {
1010

1111
template<typename WrapType, typename UVType>
1212
class ConnectionWrap {
13+
public:
14+
UVType* UVHandle();
15+
1316
protected:
1417
static void OnConnection(uv_stream_t* handle, int status);
18+
UVType uvhandle_;
1519
};
1620

1721

‎src/node_internals.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ void GetSockOrPeerName(const v8::FunctionCallbackInfo<v8::Value>& args) {
8686
sockaddr_storage storage;
8787
int addrlen = sizeof(storage);
8888
sockaddr* const addr = reinterpret_cast<sockaddr*>(&storage);
89-
const int err = F(&wrap->handle_, addr, &addrlen);
89+
const int err = F(&wrap->uvhandle_, addr, &addrlen);
9090
if (err == 0)
9191
AddressToJS(wrap->env(), addr, args[0].As<v8::Object>());
9292
args.GetReturnValue().Set(err);

‎src/pipe_wrap.cc‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,6 @@ static void NewPipeConnectWrap(const FunctionCallbackInfo<Value>& args) {
5151
}
5252

5353

54-
uv_pipe_t* PipeWrap::UVHandle() {
55-
return &handle_;
56-
}
57-
58-
5954
Local<Object> PipeWrap::Instantiate(Environment* env, AsyncWrap* parent) {
6055
EscapableHandleScope handle_scope(env->isolate());
6156
CHECK_EQ(false, env->pipe_constructor_template().IsEmpty());
@@ -127,10 +122,10 @@ PipeWrap::PipeWrap(Environment* env,
127122
AsyncWrap* parent)
128123
: StreamWrap(env,
129124
object,
130-
reinterpret_cast<uv_stream_t*>(&handle_),
125+
reinterpret_cast<uv_stream_t*>(&uvhandle_),
131126
AsyncWrap::PROVIDER_PIPEWRAP,
132127
parent) {
133-
int r = uv_pipe_init(env->event_loop(), &handle_, ipc);
128+
int r = uv_pipe_init(env->event_loop(), &uvhandle_, ipc);
134129
CHECK_EQ(r, 0); // How do we proxy this error up to javascript?
135130
// Suggestion: uv_pipe_init() returns void.
136131
UpdateWriteQueueSize();
@@ -141,7 +136,7 @@ void PipeWrap::Bind(const FunctionCallbackInfo<Value>& args) {
141136
PipeWrap* wrap;
142137
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
143138
node::Utf8Value name(args.GetIsolate(), args[0]);
144-
int err = uv_pipe_bind(&wrap->handle_, *name);
139+
int err = uv_pipe_bind(&wrap->uvhandle_, *name);
145140
args.GetReturnValue().Set(err);
146141
}
147142

@@ -151,7 +146,7 @@ void PipeWrap::SetPendingInstances(const FunctionCallbackInfo<Value>& args) {
151146
PipeWrap* wrap;
152147
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
153148
int instances = args[0]->Int32Value();
154-
uv_pipe_pending_instances(&wrap->handle_, instances);
149+
uv_pipe_pending_instances(&wrap->uvhandle_, instances);
155150
}
156151
#endif
157152

@@ -160,7 +155,7 @@ void PipeWrap::Listen(const FunctionCallbackInfo<Value>& args) {
160155
PipeWrap* wrap;
161156
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
162157
int backlog = args[0]->Int32Value();
163-
int err = uv_listen(reinterpret_cast<uv_stream_t*>(&wrap->handle_),
158+
int err = uv_listen(reinterpret_cast<uv_stream_t*>(&wrap->uvhandle_),
164159
backlog,
165160
OnConnection);
166161
args.GetReturnValue().Set(err);
@@ -213,7 +208,7 @@ void PipeWrap::Open(const FunctionCallbackInfo<Value>& args) {
213208

214209
int fd = args[0]->Int32Value();
215210

216-
int err = uv_pipe_open(&wrap->handle_, fd);
211+
int err = uv_pipe_open(&wrap->uvhandle_, fd);
217212

218213
if (err != 0)
219214
env->isolate()->ThrowException(UVException(err, "uv_pipe_open"));
@@ -234,7 +229,7 @@ void PipeWrap::Connect(const FunctionCallbackInfo<Value>& args) {
234229

235230
PipeConnectWrap* req_wrap = new PipeConnectWrap(env, req_wrap_obj);
236231
uv_pipe_connect(&req_wrap->req_,
237-
&wrap->handle_,
232+
&wrap->uvhandle_,
238233
*name,
239234
AfterConnect);
240235
req_wrap->Dispatched();

‎src/pipe_wrap.h‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@
1010

1111
namespace node {
1212

13-
class PipeWrap : public StreamWrap, ConnectionWrap<PipeWrap, uv_pipe_t> {
13+
class PipeWrap : public StreamWrap, public ConnectionWrap<PipeWrap, uv_pipe_t> {
1414
public:
15-
uv_pipe_t* UVHandle();
16-
1715
static v8::Local<v8::Object> Instantiate(Environment* env, AsyncWrap* parent);
1816
static void Initialize(v8::Local<v8::Object> target,
1917
v8::Local<v8::Value> unused,
@@ -40,8 +38,6 @@ class PipeWrap : public StreamWrap, ConnectionWrap<PipeWrap, uv_pipe_t> {
4038
#endif
4139

4240
static void AfterConnect(uv_connect_t* req, int status);
43-
44-
uv_pipe_t handle_;
4541
};
4642

4743

‎src/tcp_wrap.cc‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,6 @@ void TCPWrap::Initialize(Local<Object> target,
121121
}
122122

123123

124-
uv_tcp_t* TCPWrap::UVHandle() {
125-
return &handle_;
126-
}
127-
128-
129124
void TCPWrap::New(const FunctionCallbackInfo<Value>& args) {
130125
// This constructor should not be exposed to public javascript.
131126
// Therefore we assert that we are not trying to call this as a
@@ -148,10 +143,10 @@ void TCPWrap::New(const FunctionCallbackInfo<Value>& args) {
148143
TCPWrap::TCPWrap(Environment* env, Local<Object> object, AsyncWrap* parent)
149144
: StreamWrap(env,
150145
object,
151-
reinterpret_cast<uv_stream_t*>(&handle_),
146+
reinterpret_cast<uv_stream_t*>(&uvhandle_),
152147
AsyncWrap::PROVIDER_TCPWRAP,
153148
parent) {
154-
int r = uv_tcp_init(env->event_loop(), &handle_);
149+
int r = uv_tcp_init(env->event_loop(), &uvhandle_);
155150
CHECK_EQ(r, 0); // How do we proxy this error up to javascript?
156151
// Suggestion: uv_tcp_init() returns void.
157152
UpdateWriteQueueSize();
@@ -169,7 +164,7 @@ void TCPWrap::SetNoDelay(const FunctionCallbackInfo<Value>& args) {
169164
args.Holder(),
170165
args.GetReturnValue().Set(UV_EBADF));
171166
int enable = static_cast<int>(args[0]->BooleanValue());
172-
int err = uv_tcp_nodelay(&wrap->handle_, enable);
167+
int err = uv_tcp_nodelay(&wrap->uvhandle_, enable);
173168
args.GetReturnValue().Set(err);
174169
}
175170

@@ -181,7 +176,7 @@ void TCPWrap::SetKeepAlive(const FunctionCallbackInfo<Value>& args) {
181176
args.GetReturnValue().Set(UV_EBADF));
182177
int enable = args[0]->Int32Value();
183178
unsigned int delay = args[1]->Uint32Value();
184-
int err = uv_tcp_keepalive(&wrap->handle_, enable, delay);
179+
int err = uv_tcp_keepalive(&wrap->uvhandle_, enable, delay);
185180
args.GetReturnValue().Set(err);
186181
}
187182

@@ -193,7 +188,7 @@ void TCPWrap::SetSimultaneousAccepts(const FunctionCallbackInfo<Value>& args) {
193188
args.Holder(),
194189
args.GetReturnValue().Set(UV_EBADF));
195190
bool enable = args[0]->BooleanValue();
196-
int err = uv_tcp_simultaneous_accepts(&wrap->handle_, enable);
191+
int err = uv_tcp_simultaneous_accepts(&wrap->uvhandle_, enable);
197192
args.GetReturnValue().Set(err);
198193
}
199194
#endif
@@ -205,7 +200,7 @@ void TCPWrap::Open(const FunctionCallbackInfo<Value>& args) {
205200
args.Holder(),
206201
args.GetReturnValue().Set(UV_EBADF));
207202
int fd = static_cast<int>(args[0]->IntegerValue());
208-
uv_tcp_open(&wrap->handle_, fd);
203+
uv_tcp_open(&wrap->uvhandle_, fd);
209204
}
210205

211206

@@ -219,7 +214,7 @@ void TCPWrap::Bind(const FunctionCallbackInfo<Value>& args) {
219214
sockaddr_in addr;
220215
int err = uv_ip4_addr(*ip_address, port, &addr);
221216
if (err == 0) {
222-
err = uv_tcp_bind(&wrap->handle_,
217+
err = uv_tcp_bind(&wrap->uvhandle_,
223218
reinterpret_cast<const sockaddr*>(&addr),
224219
0);
225220
}
@@ -237,7 +232,7 @@ void TCPWrap::Bind6(const FunctionCallbackInfo<Value>& args) {
237232
sockaddr_in6 addr;
238233
int err = uv_ip6_addr(*ip6_address, port, &addr);
239234
if (err == 0) {
240-
err = uv_tcp_bind(&wrap->handle_,
235+
err = uv_tcp_bind(&wrap->uvhandle_,
241236
reinterpret_cast<const sockaddr*>(&addr),
242237
0);
243238
}
@@ -251,7 +246,7 @@ void TCPWrap::Listen(const FunctionCallbackInfo<Value>& args) {
251246
args.Holder(),
252247
args.GetReturnValue().Set(UV_EBADF));
253248
int backlog = args[0]->Int32Value();
254-
int err = uv_listen(reinterpret_cast<uv_stream_t*>(&wrap->handle_),
249+
int err = uv_listen(reinterpret_cast<uv_stream_t*>(&wrap->uvhandle_),
255250
backlog,
256251
OnConnection);
257252
args.GetReturnValue().Set(err);
@@ -308,7 +303,7 @@ void TCPWrap::Connect(const FunctionCallbackInfo<Value>& args) {
308303
if (err == 0) {
309304
TCPConnectWrap* req_wrap = new TCPConnectWrap(env, req_wrap_obj);
310305
err = uv_tcp_connect(&req_wrap->req_,
311-
&wrap->handle_,
306+
&wrap->uvhandle_,
312307
reinterpret_cast<const sockaddr*>(&addr),
313308
AfterConnect);
314309
req_wrap->Dispatched();
@@ -342,7 +337,7 @@ void TCPWrap::Connect6(const FunctionCallbackInfo<Value>& args) {
342337
if (err == 0) {
343338
TCPConnectWrap* req_wrap = new TCPConnectWrap(env, req_wrap_obj);
344339
err = uv_tcp_connect(&req_wrap->req_,
345-
&wrap->handle_,
340+
&wrap->uvhandle_,
346341
reinterpret_cast<const sockaddr*>(&addr),
347342
AfterConnect);
348343
req_wrap->Dispatched();

‎src/tcp_wrap.h‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,13 @@
1010

1111
namespace node {
1212

13-
class TCPWrap : public StreamWrap, ConnectionWrap<TCPWrap, uv_tcp_t> {
13+
class TCPWrap : public StreamWrap, public ConnectionWrap<TCPWrap, uv_tcp_t> {
1414
public:
1515
static v8::Local<v8::Object> Instantiate(Environment* env, AsyncWrap* parent);
1616
static void Initialize(v8::Local<v8::Object> target,
1717
v8::Local<v8::Value> unused,
1818
v8::Local<v8::Context> context);
1919

20-
uv_tcp_t* UVHandle();
21-
2220
size_t self_size() const override { return sizeof(*this); }
2321

2422
private:
@@ -48,8 +46,6 @@ class TCPWrap : public StreamWrap, ConnectionWrap<TCPWrap, uv_tcp_t> {
4846
#endif
4947

5048
static void AfterConnect(uv_connect_t* req, int status);
51-
52-
uv_tcp_t handle_;
5349
};
5450

5551

‎src/udp_wrap.cc‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ static void NewSendWrap(const FunctionCallbackInfo<Value>& args) {
6565
UDPWrap::UDPWrap(Environment* env, Local<Object> object, AsyncWrap* parent)
6666
: HandleWrap(env,
6767
object,
68-
reinterpret_cast<uv_handle_t*>(&handle_),
68+
reinterpret_cast<uv_handle_t*>(&uvhandle_),
6969
AsyncWrap::PROVIDER_UDPWRAP) {
70-
int r = uv_udp_init(env->event_loop(), &handle_);
70+
int r = uv_udp_init(env->event_loop(), &uvhandle_);
7171
CHECK_EQ(r, 0); // can't fail anyway
7272
}
7373

@@ -144,7 +144,7 @@ void UDPWrap::GetFD(Local<String>, const PropertyCallbackInfo<Value>& args) {
144144
HandleScope scope(args.GetIsolate());
145145
UDPWrap* wrap = Unwrap<UDPWrap>(args.Holder());
146146
if (wrap != nullptr)
147-
uv_fileno(reinterpret_cast<uv_handle_t*>(&wrap->handle_), &fd);
147+
uv_fileno(reinterpret_cast<uv_handle_t*>(&wrap->uvhandle_), &fd);
148148
#endif
149149
args.GetReturnValue().Set(fd);
150150
}
@@ -178,7 +178,7 @@ void UDPWrap::DoBind(const FunctionCallbackInfo<Value>& args, int family) {
178178
}
179179

180180
if (err == 0) {
181-
err = uv_udp_bind(&wrap->handle_,
181+
err = uv_udp_bind(&wrap->uvhandle_,
182182
reinterpret_cast<const sockaddr*>(&addr),
183183
flags);
184184
}
@@ -202,7 +202,7 @@ void UDPWrap::Bind6(const FunctionCallbackInfo<Value>& args) {
202202
UDPWrap* wrap = Unwrap<UDPWrap>(args.Holder()); \
203203
CHECK_EQ(args.Length(), 1); \
204204
int flag = args[0]->Int32Value(); \
205-
int err = wrap == nullptr ? UV_EBADF : fn(&wrap->handle_, flag); \
205+
int err = wrap == nullptr ? UV_EBADF : fn(&wrap->uvhandle_, flag); \
206206
args.GetReturnValue().Set(err); \
207207
}
208208

@@ -231,7 +231,7 @@ void UDPWrap::SetMembership(const FunctionCallbackInfo<Value>& args,
231231
iface_cstr = nullptr;
232232
}
233233

234-
int err = uv_udp_set_membership(&wrap->handle_,
234+
int err = uv_udp_set_membership(&wrap->uvhandle_,
235235
*address,
236236
iface_cstr,
237237
membership);
@@ -314,7 +314,7 @@ void UDPWrap::DoSend(const FunctionCallbackInfo<Value>& args, int family) {
314314

315315
if (err == 0) {
316316
err = uv_udp_send(&req_wrap->req_,
317-
&wrap->handle_,
317+
&wrap->uvhandle_,
318318
bufs,
319319
count,
320320
reinterpret_cast<const sockaddr*>(&addr),
@@ -348,7 +348,7 @@ void UDPWrap::RecvStart(const FunctionCallbackInfo<Value>& args) {
348348
ASSIGN_OR_RETURN_UNWRAP(&wrap,
349349
args.Holder(),
350350
args.GetReturnValue().Set(UV_EBADF));
351-
int err = uv_udp_recv_start(&wrap->handle_, OnAlloc, OnRecv);
351+
int err = uv_udp_recv_start(&wrap->uvhandle_, OnAlloc, OnRecv);
352352
// UV_EALREADY means that the socket is already bound but that's okay
353353
if (err == UV_EALREADY)
354354
err = 0;
@@ -361,7 +361,7 @@ void UDPWrap::RecvStop(const FunctionCallbackInfo<Value>& args) {
361361
ASSIGN_OR_RETURN_UNWRAP(&wrap,
362362
args.Holder(),
363363
args.GetReturnValue().Set(UV_EBADF));
364-
int r = uv_udp_recv_stop(&wrap->handle_);
364+
int r = uv_udp_recv_stop(&wrap->uvhandle_);
365365
args.GetReturnValue().Set(r);
366366
}
367367

@@ -446,7 +446,7 @@ Local<Object> UDPWrap::Instantiate(Environment* env, AsyncWrap* parent) {
446446

447447

448448
uv_udp_t* UDPWrap::UVHandle() {
449-
return &handle_;
449+
return &uvhandle_;
450450
}
451451

452452

‎src/udp_wrap.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class UDPWrap: public HandleWrap {
6767
const struct sockaddr* addr,
6868
unsigned int flags);
6969

70-
uv_udp_t handle_;
70+
uv_udp_t uvhandle_;
7171
};
7272

7373
} // namespace node

0 commit comments

Comments
 (0)