stream_wrap: use getters, not direct field access

Hide member fields behind getters. Make the fields themselves const
in the sense that the pointer is non-assignable - the pointed to object
remains mutable.

Makes reasoning about lifecycle and mutability a little easier.

diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
index 17c8b89..10e70bf 100644 (file)
--- a/src/stream_wrap.cc
+++ b/src/stream_wrap.cc
@@ -76,9 +76,9 @@ void StreamWrap::Initialize(Handle<Object> target) {
 
 StreamWrap::StreamWrap(Handle<Object> object, uv_stream_t* stream)
     : HandleWrap(object, reinterpret_cast<uv_handle_t*>(stream)),
 
 StreamWrap::StreamWrap(Handle<Object> object, uv_stream_t* stream)
     : HandleWrap(object, reinterpret_cast<uv_handle_t*>(stream)),

-      default_callbacks_(this) {
-  stream_ = stream;
-  callbacks_ = &default_callbacks_;
+      stream_(stream),
+      default_callbacks_(this),
+      callbacks_(&default_callbacks_) {

 }
 
 
 }
 
 


@@ -87,7 +87,9 @@ void StreamWrap::GetFD(Local<String>, const PropertyCallbackInfo<Value>& args) {
   HandleScope scope(node_isolate);
   UNWRAP_NO_ABORT(StreamWrap)
   int fd = -1;
   HandleScope scope(node_isolate);
   UNWRAP_NO_ABORT(StreamWrap)
   int fd = -1;

-  if (wrap != NULL && wrap->stream_ != NULL) fd = wrap->stream_->io_watcher.fd;
+  if (wrap != NULL && wrap->stream() != NULL) {
+    fd = wrap->stream()->io_watcher.fd;
+  }

   args.GetReturnValue().Set(fd);
 #endif
 }
   args.GetReturnValue().Set(fd);
 #endif
 }


@@ -96,7 +98,7 @@ void StreamWrap::GetFD(Local<String>, const PropertyCallbackInfo<Value>& args) {
 void StreamWrap::UpdateWriteQueueSize() {
   HandleScope scope(node_isolate);
   object()->Set(write_queue_size_sym,
 void StreamWrap::UpdateWriteQueueSize() {
   HandleScope scope(node_isolate);
   object()->Set(write_queue_size_sym,

-                Integer::New(stream_->write_queue_size, node_isolate));
+                Integer::New(stream()->write_queue_size, node_isolate));

 }
 
 
 }
 
 


@@ -105,13 +107,13 @@ void StreamWrap::ReadStart(const FunctionCallbackInfo<Value>& args) {
 
   UNWRAP(StreamWrap)
 
 
   UNWRAP(StreamWrap)
 

-  bool ipc_pipe = wrap->stream_->type == UV_NAMED_PIPE &&
-                  reinterpret_cast<uv_pipe_t*>(wrap->stream_)->ipc;
+  bool ipc_pipe = wrap->stream()->type == UV_NAMED_PIPE &&
+                  reinterpret_cast<uv_pipe_t*>(wrap->stream())->ipc;

   int err;
   if (ipc_pipe) {
   int err;
   if (ipc_pipe) {

-    err = uv_read2_start(wrap->stream_, OnAlloc, OnRead2);
+    err = uv_read2_start(wrap->stream(), OnAlloc, OnRead2);

   } else {
   } else {

-    err = uv_read_start(wrap->stream_, OnAlloc, OnRead);
+    err = uv_read_start(wrap->stream(), OnAlloc, OnRead);

   }
 
   args.GetReturnValue().Set(err);
   }
 
   args.GetReturnValue().Set(err);


@@ -123,16 +125,15 @@ void StreamWrap::ReadStop(const FunctionCallbackInfo<Value>& args) {
 
   UNWRAP(StreamWrap)
 
 
   UNWRAP(StreamWrap)
 

-  int err = uv_read_stop(wrap->stream_);
+  int err = uv_read_stop(wrap->stream());

   args.GetReturnValue().Set(err);
 }
 
 
 uv_buf_t StreamWrap::OnAlloc(uv_handle_t* handle, size_t suggested_size) {
   StreamWrap* wrap = static_cast<StreamWrap*>(handle->data);
   args.GetReturnValue().Set(err);
 }
 
 
 uv_buf_t StreamWrap::OnAlloc(uv_handle_t* handle, size_t suggested_size) {
   StreamWrap* wrap = static_cast<StreamWrap*>(handle->data);

-  assert(wrap->stream_ == reinterpret_cast<uv_stream_t*>(handle));
-
-  return wrap->callbacks_->DoAlloc(handle, suggested_size);
+  assert(wrap->stream() == reinterpret_cast<uv_stream_t*>(handle));
+  return wrap->callbacks()->DoAlloc(handle, suggested_size);

 }
 
 
 }
 
 


@@ -171,14 +172,14 @@ void StreamWrap::OnReadCommon(uv_stream_t* handle,
   assert(wrap->persistent().IsEmpty() == false);
 
   if (nread > 0) {
   assert(wrap->persistent().IsEmpty() == false);
 
   if (nread > 0) {

-    if (wrap->stream_->type == UV_TCP) {
+    if (wrap->stream()->type == UV_TCP) {

       NODE_COUNT_NET_BYTES_RECV(nread);
       NODE_COUNT_NET_BYTES_RECV(nread);

-    } else if (wrap->stream_->type == UV_NAMED_PIPE) {
+    } else if (wrap->stream()->type == UV_NAMED_PIPE) {

       NODE_COUNT_PIPE_BYTES_RECV(nread);
     }
   }
 
       NODE_COUNT_PIPE_BYTES_RECV(nread);
     }
   }
 

-  wrap->callbacks_->DoRead(handle, nread, buf, pending);
+  wrap->callbacks()->DoRead(handle, nread, buf, pending);

 }
 
 
 }
 
 


@@ -224,11 +225,11 @@ void StreamWrap::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
   uv_buf_t buf;
   WriteBuffer(buf_obj, &buf);
 
   uv_buf_t buf;
   WriteBuffer(buf_obj, &buf);
 

-  int err = wrap->callbacks_->DoWrite(req_wrap,
-                                      &buf,
-                                      1,
-                                      NULL,
-                                      StreamWrap::AfterWrite);
+  int err = wrap->callbacks()->DoWrite(req_wrap,
+                                       &buf,
+                                       1,
+                                       NULL,
+                                       StreamWrap::AfterWrite);

   req_wrap->Dispatched();
   req_wrap_obj->Set(bytes_sym, Integer::NewFromUnsigned(length, node_isolate));
 
   req_wrap->Dispatched();
   req_wrap_obj->Set(bytes_sym, Integer::NewFromUnsigned(length, node_isolate));
 


@@ -284,15 +285,15 @@ void StreamWrap::WriteStringImpl(const FunctionCallbackInfo<Value>& args) {
   buf.base = data;
   buf.len = data_size;
 
   buf.base = data;
   buf.len = data_size;
 

-  bool ipc_pipe = wrap->stream_->type == UV_NAMED_PIPE &&
-                  reinterpret_cast<uv_pipe_t*>(wrap->stream_)->ipc;
+  bool ipc_pipe = wrap->stream()->type == UV_NAMED_PIPE &&
+                  reinterpret_cast<uv_pipe_t*>(wrap->stream())->ipc;

 
   if (!ipc_pipe) {
 
   if (!ipc_pipe) {

-    err = wrap->callbacks_->DoWrite(req_wrap,
-                                    &buf,
-                                    1,
-                                    NULL,
-                                    StreamWrap::AfterWrite);
+    err = wrap->callbacks()->DoWrite(req_wrap,
+                                     &buf,
+                                     1,
+                                     NULL,
+                                     StreamWrap::AfterWrite);

   } else {
     uv_handle_t* send_handle = NULL;
 
   } else {
     uv_handle_t* send_handle = NULL;
 


@@ -312,11 +313,11 @@ void StreamWrap::WriteStringImpl(const FunctionCallbackInfo<Value>& args) {
       req_wrap->object()->Set(handle_sym, send_handle_obj);
     }
 
       req_wrap->object()->Set(handle_sym, send_handle_obj);
     }
 

-    err = wrap->callbacks_->DoWrite(req_wrap,
-                                    &buf,
-                                    1,
-                                    reinterpret_cast<uv_stream_t*>(send_handle),
-                                    StreamWrap::AfterWrite);
+    err = wrap->callbacks()->DoWrite(req_wrap,
+                                     &buf,
+                                     1,
+                                     reinterpret_cast<uv_stream_t*>(send_handle),
+                                     StreamWrap::AfterWrite);

   }
 
   req_wrap->Dispatched();
   }
 
   req_wrap->Dispatched();


@@ -407,11 +408,11 @@ void StreamWrap::Writev(const FunctionCallbackInfo<Value>& args) {
     bytes += str_size;
   }
 
     bytes += str_size;
   }
 

-  int err = wrap->callbacks_->DoWrite(req_wrap,
-                                      bufs,
-                                      count,
-                                      NULL,
-                                      StreamWrap::AfterWrite);
+  int err = wrap->callbacks()->DoWrite(req_wrap,
+                                       bufs,
+                                       count,
+                                       NULL,
+                                       StreamWrap::AfterWrite);

 
   // Deallocate space
   if (bufs != bufs_)
 
   // Deallocate space
   if (bufs != bufs_)


@@ -446,7 +447,7 @@ void StreamWrap::WriteUcs2String(const FunctionCallbackInfo<Value>& args) {
 
 void StreamWrap::AfterWrite(uv_write_t* req, int status) {
   WriteWrap* req_wrap = container_of(req, WriteWrap, req_);
 
 void StreamWrap::AfterWrite(uv_write_t* req, int status) {
   WriteWrap* req_wrap = container_of(req, WriteWrap, req_);

-  StreamWrap* wrap = req_wrap->wrap_;
+  StreamWrap* wrap = req_wrap->wrap();

 
   HandleScope scope(node_isolate);
 
 
   HandleScope scope(node_isolate);
 


@@ -460,7 +461,7 @@ void StreamWrap::AfterWrite(uv_write_t* req, int status) {
     req_wrap_obj->Delete(handle_sym);
   }
 
     req_wrap_obj->Delete(handle_sym);
   }
 

-  wrap->callbacks_->AfterWrite(req_wrap);
+  wrap->callbacks()->AfterWrite(req_wrap);

 
   Local<Value> argv[] = {
     Integer::New(status, node_isolate),
 
   Local<Value> argv[] = {
     Integer::New(status, node_isolate),


@@ -484,7 +485,7 @@ void StreamWrap::Shutdown(const FunctionCallbackInfo<Value>& args) {
   Local<Object> req_wrap_obj = args[0].As<Object>();
 
   ShutdownWrap* req_wrap = new ShutdownWrap(req_wrap_obj);
   Local<Object> req_wrap_obj = args[0].As<Object>();
 
   ShutdownWrap* req_wrap = new ShutdownWrap(req_wrap_obj);

-  int err = wrap->callbacks_->DoShutdown(req_wrap, AfterShutdown);
+  int err = wrap->callbacks()->DoShutdown(req_wrap, AfterShutdown);

   req_wrap->Dispatched();
   if (err) delete req_wrap;
   args.GetReturnValue().Set(err);
   req_wrap->Dispatched();
   if (err) delete req_wrap;
   args.GetReturnValue().Set(err);


@@ -521,30 +522,30 @@ int StreamWrapCallbacks::DoWrite(WriteWrap* w,
                                  uv_write_cb cb) {
   int r;
   if (send_handle == NULL) {
                                  uv_write_cb cb) {
   int r;
   if (send_handle == NULL) {

-    r = uv_write(&w->req_, wrap_->stream_, bufs, count, cb);
+    r = uv_write(&w->req_, wrap()->stream(), bufs, count, cb);

   } else {
   } else {

-    r = uv_write2(&w->req_, wrap_->stream_, bufs, count, send_handle, cb);
+    r = uv_write2(&w->req_, wrap()->stream(), bufs, count, send_handle, cb);

   }
 
   if (!r) {
     size_t bytes = 0;
     for (size_t i = 0; i < count; i++)
       bytes += bufs[i].len;
   }
 
   if (!r) {
     size_t bytes = 0;
     for (size_t i = 0; i < count; i++)
       bytes += bufs[i].len;

-    if (wrap_->stream_->type == UV_TCP) {
+    if (wrap()->stream()->type == UV_TCP) {

       NODE_COUNT_NET_BYTES_SENT(bytes);
       NODE_COUNT_NET_BYTES_SENT(bytes);

-    } else if (wrap_->stream_->type == UV_NAMED_PIPE) {
+    } else if (wrap()->stream()->type == UV_NAMED_PIPE) {

       NODE_COUNT_PIPE_BYTES_SENT(bytes);
     }
   }
 
       NODE_COUNT_PIPE_BYTES_SENT(bytes);
     }
   }
 

-  wrap_->UpdateWriteQueueSize();
+  wrap()->UpdateWriteQueueSize();

 
   return r;
 }
 
 
 void StreamWrapCallbacks::AfterWrite(WriteWrap* w) {
 
   return r;
 }
 
 
 void StreamWrapCallbacks::AfterWrite(WriteWrap* w) {

-  wrap_->UpdateWriteQueueSize();
+  wrap()->UpdateWriteQueueSize();

 }
 
 
 }
 
 


@@ -603,17 +604,17 @@ void StreamWrapCallbacks::DoRead(uv_stream_t* handle,
     argv[2] = pending_obj;
   }
 
     argv[2] = pending_obj;
   }
 

-  MakeCallback(wrap_->object(), onread_sym, ARRAY_SIZE(argv), argv);
+  MakeCallback(wrap()->object(), onread_sym, ARRAY_SIZE(argv), argv);

 }
 
 
 int StreamWrapCallbacks::DoShutdown(ShutdownWrap* req_wrap, uv_shutdown_cb cb) {
 }
 
 
 int StreamWrapCallbacks::DoShutdown(ShutdownWrap* req_wrap, uv_shutdown_cb cb) {

-  return uv_shutdown(&req_wrap->req_, wrap_->stream_, cb);
+  return uv_shutdown(&req_wrap->req_, wrap()->stream(), cb);

 }
 
 
 Handle<Object> StreamWrapCallbacks::Self() {
 }
 
 
 Handle<Object> StreamWrapCallbacks::Self() {

-  return wrap_->object();
+  return wrap()->object();

 }
 
 }  // namespace node
 }
 
 }  // namespace node

diff --git a/src/stream_wrap.h b/src/stream_wrap.h
index 98d22cb..25aff8b 100644 (file)
--- a/src/stream_wrap.h
+++ b/src/stream_wrap.h
@@ -37,9 +37,9 @@ typedef class ReqWrap<uv_shutdown_t> ShutdownWrap;
 
 class WriteWrap: public ReqWrap<uv_write_t> {
  public:
 
 class WriteWrap: public ReqWrap<uv_write_t> {
  public:

-  explicit WriteWrap(v8::Local<v8::Object> obj, StreamWrap* wrap)
-      : ReqWrap<uv_write_t>(obj) {
-    wrap_ = wrap;
+  WriteWrap(v8::Local<v8::Object> obj, StreamWrap* wrap)
+      : ReqWrap<uv_write_t>(obj)
+      , wrap_(wrap) {

   }
 
   void* operator new(size_t size, char* storage) { return storage; }
   }
 
   void* operator new(size_t size, char* storage) { return storage; }


@@ -48,13 +48,17 @@ class WriteWrap: public ReqWrap<uv_write_t> {
   // we don't use exceptions in node.
   void operator delete(void* ptr, char* storage) { assert(0); }
 
   // we don't use exceptions in node.
   void operator delete(void* ptr, char* storage) { assert(0); }
 

-  StreamWrap* wrap_;
+  inline StreamWrap* wrap() const {
+    return wrap_;
+  }

 
 

- protected:
+ private:

   // People should not be using the non-placement new and delete operator on a
   // WriteWrap. Ensure this never happens.
   void* operator new(size_t size) { assert(0); }
   void operator delete(void* ptr) { assert(0); }
   // People should not be using the non-placement new and delete operator on a
   // WriteWrap. Ensure this never happens.
   void* operator new(size_t size) { assert(0); }
   void operator delete(void* ptr) { assert(0); }

+
+  StreamWrap* const wrap_;

 };
 
 // Overridable callbacks' types
 };
 
 // Overridable callbacks' types


@@ -63,7 +67,7 @@ class StreamWrapCallbacks {
   explicit StreamWrapCallbacks(StreamWrap* wrap) : wrap_(wrap) {
   }
 
   explicit StreamWrapCallbacks(StreamWrap* wrap) : wrap_(wrap) {
   }
 

-  explicit StreamWrapCallbacks(StreamWrapCallbacks* old) : wrap_(old->wrap_) {
+  explicit StreamWrapCallbacks(StreamWrapCallbacks* old) : wrap_(old->wrap()) {

   }
 
   virtual ~StreamWrapCallbacks() {
   }
 
   virtual ~StreamWrapCallbacks() {


@@ -85,13 +89,16 @@ class StreamWrapCallbacks {
   v8::Handle<v8::Object> Self();
 
  protected:
   v8::Handle<v8::Object> Self();
 
  protected:

-  StreamWrap* wrap_;
+  inline StreamWrap* wrap() const {
+    return wrap_;
+  }
+
+ private:
+  StreamWrap* const wrap_;

 };
 
 class StreamWrap : public HandleWrap {
  public:
 };
 
 class StreamWrap : public HandleWrap {
  public:

-  uv_stream_t* GetStream() { return stream_; }
-

   void OverrideCallbacks(StreamWrapCallbacks* callbacks) {
     StreamWrapCallbacks* old = callbacks_;
     callbacks_ = callbacks;
   void OverrideCallbacks(StreamWrapCallbacks* callbacks) {
     StreamWrapCallbacks* old = callbacks_;
     callbacks_ = callbacks;


@@ -99,10 +106,6 @@ class StreamWrap : public HandleWrap {
       delete old;
   }
 
       delete old;
   }
 

-  StreamWrapCallbacks* GetCallbacks() {
-    return callbacks_;
-  }
-

   static void Initialize(v8::Handle<v8::Object> target);
 
   static void GetFD(v8::Local<v8::String>,
   static void Initialize(v8::Handle<v8::Object> target);
 
   static void GetFD(v8::Local<v8::String>,


@@ -119,19 +122,26 @@ class StreamWrap : public HandleWrap {
   static void WriteUtf8String(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void WriteUcs2String(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   static void WriteUtf8String(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void WriteUcs2String(const v8::FunctionCallbackInfo<v8::Value>& args);
 

-  // Overridable callbacks
-  StreamWrapCallbacks* callbacks_;
+  inline StreamWrapCallbacks* callbacks() const {
+    return callbacks_;
+  }
+
+  inline uv_stream_t* stream() const {
+    return stream_;
+  }

 
  protected:
   static size_t WriteBuffer(v8::Handle<v8::Value> val, uv_buf_t* buf);
 
   StreamWrap(v8::Handle<v8::Object> object, uv_stream_t* stream);
 
  protected:
   static size_t WriteBuffer(v8::Handle<v8::Value> val, uv_buf_t* buf);
 
   StreamWrap(v8::Handle<v8::Object> object, uv_stream_t* stream);

+

   ~StreamWrap() {
     if (callbacks_ != &default_callbacks_) {
       delete callbacks_;
       callbacks_ = NULL;
     }
   }
   ~StreamWrap() {
     if (callbacks_ != &default_callbacks_) {
       delete callbacks_;
       callbacks_ = NULL;
     }
   }

+

   void StateChange() { }
   void UpdateWriteQueueSize();
 
   void StateChange() { }
   void UpdateWriteQueueSize();
 


@@ -150,9 +160,10 @@ class StreamWrap : public HandleWrap {
   template <enum encoding encoding>
   static void WriteStringImpl(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   template <enum encoding encoding>
   static void WriteStringImpl(const v8::FunctionCallbackInfo<v8::Value>& args);
 

-  uv_stream_t* stream_;
-
+  uv_stream_t* const stream_;

   StreamWrapCallbacks default_callbacks_;
   StreamWrapCallbacks default_callbacks_;

+  StreamWrapCallbacks* callbacks_;  // Overridable callbacks
+

   friend class StreamWrapCallbacks;
 };
 
   friend class StreamWrapCallbacks;
 };
 

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index b3172a1..66dfd6e 100644 (file)
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -316,7 +316,7 @@ void TLSCallbacks::Wrap(const FunctionCallbackInfo<Value>& args) {
 
   TLSCallbacks* callbacks = NULL;
   WITH_GENERIC_STREAM(stream, {
 
   TLSCallbacks* callbacks = NULL;
   WITH_GENERIC_STREAM(stream, {

-    callbacks = new TLSCallbacks(kind, sc, wrap->GetCallbacks());
+    callbacks = new TLSCallbacks(kind, sc, wrap->callbacks());

     wrap->OverrideCallbacks(callbacks);
   });
 
     wrap->OverrideCallbacks(callbacks);
   });
 


@@ -394,13 +394,13 @@ void TLSCallbacks::EncOut() {
 
   write_req_.data = this;
   uv_buf_t buf = uv_buf_init(data, write_size_);
 
   write_req_.data = this;
   uv_buf_t buf = uv_buf_init(data, write_size_);

-  int r = uv_write(&write_req_, wrap_->GetStream(), &buf, 1, EncOutCb);
+  int r = uv_write(&write_req_, wrap()->stream(), &buf, 1, EncOutCb);

 
   // Ignore errors, this should be already handled in js
   if (!r) {
 
   // Ignore errors, this should be already handled in js
   if (!r) {

-    if (wrap_->GetStream()->type == UV_TCP) {
+    if (wrap()->stream()->type == UV_TCP) {

       NODE_COUNT_NET_BYTES_SENT(write_size_);
       NODE_COUNT_NET_BYTES_SENT(write_size_);

-    } else if (wrap_->GetStream()->type == UV_NAMED_PIPE) {
+    } else if (wrap()->stream()->type == UV_NAMED_PIPE) {

       NODE_COUNT_PIPE_BYTES_SENT(write_size_);
     }
   }
       NODE_COUNT_PIPE_BYTES_SENT(write_size_);
     }
   }


@@ -560,7 +560,7 @@ int TLSCallbacks::DoWrite(WriteWrap* w,
     // However if there any data that should be written to socket,
     // callback should not be invoked immediately
     if (BIO_pending(enc_out_) == 0)
     // However if there any data that should be written to socket,
     // callback should not be invoked immediately
     if (BIO_pending(enc_out_) == 0)

-      return uv_write(&w->req_, wrap_->GetStream(), bufs, count, cb);
+      return uv_write(&w->req_, wrap()->stream(), bufs, count, cb);

   }
 
   QUEUE_INSERT_TAIL(&write_item_queue_, &wi->member_);
   }
 
   QUEUE_INSERT_TAIL(&write_item_queue_, &wi->member_);