From bf803f478bfdee522adf42b5c1207deb75cdb7dc Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 27 Jan 2010 15:40:09 -0800 Subject: [PATCH] Reimplment Buffers --- src/node.cc | 2 +- src/node_buffer.cc | 385 ++++++++++++++++-------------------- src/node_buffer.h | 68 +++---- src/node_http_parser.cc | 72 ++++--- src/node_io_watcher.cc | 51 ++--- src/node_net2.cc | 38 ++-- src/node_object_wrap.h | 5 +- test/mjsunit/test-buffer.js | 21 +- wscript | 2 +- 9 files changed, 293 insertions(+), 351 deletions(-) diff --git a/src/node.cc b/src/node.cc index 6c4fd51fdd..f7785ff774 100644 --- a/src/node.cc +++ b/src/node.cc @@ -989,7 +989,7 @@ static Local Load(int argc, char *argv[]) { // Initialize the C++ modules..................filename of module - InitBuffer(process); // buffer.cc + Buffer::Initialize(process); // buffer.cc IOWatcher::Initialize(process); // io_watcher.cc IdleWatcher::Initialize(process); // idle_watcher.cc Timer::Initialize(process); // timer.cc diff --git a/src/node_buffer.cc b/src/node_buffer.cc index ee9ce8406f..7ebfc089f3 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -10,254 +10,198 @@ namespace node { using namespace v8; -#define SLICE_ARGS(start_arg, end_arg) \ - if (!start_arg->IsInt32() || !end_arg->IsInt32()) { \ - return ThrowException(Exception::TypeError( \ - String::New("Bad argument."))); \ - } \ - int32_t start = start_arg->Int32Value(); \ - int32_t end = end_arg->Int32Value(); \ - if (start < 0 || end < 0) { \ - return ThrowException(Exception::TypeError( \ - String::New("Bad argument."))); \ - } +#define SLICE_ARGS(start_arg, end_arg) \ + if (!start_arg->IsInt32() || !end_arg->IsInt32()) { \ + return ThrowException(Exception::TypeError( \ + String::New("Bad argument."))); \ + } \ + int32_t start = start_arg->Int32Value(); \ + int32_t end = end_arg->Int32Value(); \ + if (start < 0 || end < 0) { \ + return ThrowException(Exception::TypeError( \ + String::New("Bad argument."))); \ + } \ + if (!(start <= end)) { \ + return ThrowException(Exception::Error( \ + String::New("Must have start <= end"))); \ + } \ + if ((size_t)end > parent->length_) { \ + return ThrowException(Exception::Error( \ + String::New("end cannot be longer than parent.length"))); \ + } -static Persistent length_symbol; -static Persistent constructor_template; -bool IsBuffer(v8::Handle val) { - if (!val->IsObject()) return false; - Local obj = val->ToObject(); - return constructor_template->HasInstance(obj); -} +static Persistent length_symbol; +Persistent Buffer::constructor_template; -/* Determines the absolute position for a relative offset */ -size_t buffer_abs_off(buffer *buffer, size_t off) { - struct buffer *root = buffer_root(buffer); - off += buffer->offset; - return MIN(root->length, off); +// Each javascript Buffer object is backed by a Blob object. +// the Blob is just a C-level chunk of bytes. +// It has a reference count. +struct Blob_ { + unsigned int refs; + size_t length; + char data[1]; +}; +typedef struct Blob_ Blob; + + +static inline Blob * blob_new(size_t length) { + size_t s = sizeof(Blob) - 1 + length; + Blob * blob = (Blob*) malloc(s); + if (!blob) return NULL; + V8::AdjustAmountOfExternalAllocatedMemory(s); + blob->length = length; + blob->refs = 0; + //fprintf(stderr, "alloc %d bytes\n", length); + return blob; } -void buffer_ref(struct buffer *buffer) { - buffer_root(buffer)->refs++; +static inline void blob_ref(Blob *blob) { + blob->refs++; } -struct buffer* BufferUnwrap(v8::Handle val) { - assert(val->IsObject()); - HandleScope scope; - Local obj = val->ToObject(); - assert(obj->InternalFieldCount() == 1); - Local ext = Local::Cast(obj->GetInternalField(0)); - return static_cast(ext->Value()); +static inline void blob_unref(Blob *blob) { + assert(blob->refs > 0); + if (--blob->refs == 0) { + //fprintf(stderr, "free %d bytes\n", blob->length); + size_t s = sizeof(Blob) - 1 + blob->length; + V8::AdjustAmountOfExternalAllocatedMemory(-s); + free(blob); + } } -static void RootWeakCallback(Persistent value, void *data) -{ - struct buffer *buffer = static_cast(data); - assert(buffer->root == NULL); // this is the root - assert(value == buffer->handle); - value.ClearWeak(); - if (buffer->refs) { - buffer->weak = true; - } else { - buffer->handle.Dispose(); - free(buffer); +// When someone calls buffer.asciiSlice, data is not copied. Instead V8 +// references in the underlying Blob with this ExternalAsciiStringResource. +class AsciiSliceExt: public String::ExternalAsciiStringResource { + friend class Buffer; + public: + AsciiSliceExt(Buffer *parent, size_t start, size_t end) { + blob_ = parent->blob(); + blob_ref(blob_); + + assert(start <= end); + length_ = end - start; + assert(length_ <= parent->length()); + data_ = parent->data() + start; } -} -void buffer_unref(struct buffer *buffer) { - struct buffer * root = buffer_root(buffer); - assert(root->refs > 0); - root->refs--; - if (root->refs == 0 && root->weak) { - root->handle.MakeWeak(root, RootWeakCallback); + ~AsciiSliceExt() { + //fprintf(stderr, "free ascii slice (%d refs left)\n", blob_->refs); + blob_unref(blob_); } -} -static void SliceWeakCallback(Persistent value, void *data) -{ - struct buffer *buffer = static_cast(data); - assert(buffer->root != NULL); // this is a slice - assert(value == buffer->handle); - buffer->handle.Dispose(); - buffer_unref(buffer->root); -} + const char* data() const { return data_; } + size_t length() const { return length_; } + + private: + const char *data_; + size_t length_; + Blob *blob_; +}; -static Handle Constructor(const Arguments &args) { +Handle Buffer::New(const Arguments &args) { HandleScope scope; - size_t length; - struct buffer *buffer; + Buffer *buffer; + if (args[0]->IsInt32()) { + // var buffer = new Buffer(1024); + size_t length = args[0]->Uint32Value(); + buffer = new Buffer(length); - if (constructor_template->HasInstance(args[0])) { - // slice slice + } else if (Buffer::HasInstance(args[0]) && args.Length() > 2) { + // var slice = new Buffer(buffer, 123, 130); + // args: parent, start, end + Buffer *parent = ObjectWrap::Unwrap(args[0]->ToObject()); SLICE_ARGS(args[1], args[2]) - - struct buffer *parent = BufferUnwrap(args[0]); - - size_t start_abs = buffer_abs_off(parent, start); - size_t end_abs = buffer_abs_off(parent, end); - assert(start_abs <= end_abs); - length = end_abs - start_abs; - - void *d = malloc(sizeof(struct buffer)); - - if (!d) { - V8::LowMemoryNotification(); - return ThrowException(Exception::Error( - String::New("Could not allocate enough memory"))); - - } - - buffer = static_cast(d); - - buffer->length = length; - buffer->offset = start_abs; - buffer->weak = false; - buffer->refs = 0; - buffer->root = buffer_root(parent); - buffer->handle = Persistent::New(args.This()); - buffer->handle.MakeWeak(buffer, SliceWeakCallback); - - buffer_ref(buffer->root); + buffer = new Buffer(parent, start, end); } else { - // Root slice - - length = args[0]->Uint32Value(); - - if (length < 1) { - return ThrowException(Exception::TypeError( - String::New("Bad argument. Length must be positive"))); - } - - // TODO alignment. modify the length? - void *d = malloc(sizeof(struct buffer) + length - 1); - - if (!d) { - V8::LowMemoryNotification(); - return ThrowException(Exception::Error( - String::New("Could not allocate enough memory"))); - } - - buffer = static_cast(d); - - buffer->offset = 0; - buffer->length = length; - buffer->weak = false; - buffer->refs = 0; - buffer->root = NULL; - buffer->handle = Persistent::New(args.This()); - buffer->handle.MakeWeak(buffer, RootWeakCallback); + return ThrowException(Exception::TypeError(String::New("Bad argument"))); } - args.This()->SetInternalField(0, v8::External::New(buffer)); - - struct buffer *root = buffer_root(buffer); + buffer->Wrap(args.This()); + args.This()->SetIndexedPropertiesToExternalArrayData((void*)buffer->data_, + kExternalUnsignedByteArray, + buffer->length_); + args.This()->Set(length_symbol, Integer::New(buffer->length_)); + return args.This(); +} - args.This()-> - SetIndexedPropertiesToExternalArrayData(&root->bytes + buffer->offset, - kExternalUnsignedByteArray, - length); - args.This()->Set(length_symbol, Integer::New(length)); +Buffer::Buffer(size_t length) : ObjectWrap() { + blob_ = blob_new(length); + length_ = length; + data_ = blob_->data; + blob_ref(blob_); - return args.This(); + V8::AdjustAmountOfExternalAllocatedMemory(sizeof(Buffer)); } -class AsciiSliceExt: public String::ExternalAsciiStringResource { - public: +Buffer::Buffer(Buffer *parent, size_t start, size_t end) : ObjectWrap() { + blob_ = parent->blob_; + assert(blob_->refs > 0); + blob_ref(blob_); - AsciiSliceExt(struct buffer *root, size_t start, size_t end) - { - data_ = root->bytes + start; - len_ = end - start; - root_ = root; - buffer_ref(root_); - } + assert(start <= end); + length_ = end - start; + assert(length_ <= parent->length_); + data_ = parent->data_ + start; - ~AsciiSliceExt() { - buffer_unref(root_); - } + V8::AdjustAmountOfExternalAllocatedMemory(sizeof(Buffer)); +} - const char* data() const { - return data_; - } - size_t length() const { - return len_; - } +Buffer::~Buffer() { + assert(blob_->refs > 0); + //fprintf(stderr, "free buffer (%d refs left)\n", blob_->refs); + blob_unref(blob_); + V8::AdjustAmountOfExternalAllocatedMemory(-sizeof(Buffer)); +} - private: - const char *data_; - size_t len_; - struct buffer *root_; -}; -static Handle AsciiSlice(const Arguments &args) { +Handle Buffer::AsciiSlice(const Arguments &args) { HandleScope scope; - + Buffer *parent = ObjectWrap::Unwrap(args.This()); SLICE_ARGS(args[0], args[1]) - - assert(args.This()->InternalFieldCount() == 1); - struct buffer *parent = BufferUnwrap(args.This()); - - size_t start_abs = buffer_abs_off(parent, start); - size_t end_abs = buffer_abs_off(parent, end); - - assert(start_abs <= end_abs); - - AsciiSliceExt *s = new AsciiSliceExt(buffer_root(parent), start_abs, end_abs); - Local string = String::NewExternal(s); - - struct buffer *root = buffer_root(parent); - assert(root->refs > 0); - + AsciiSliceExt *ext = new AsciiSliceExt(parent, start, end); + Local string = String::NewExternal(ext); + // There should be at least two references to the blob now - the parent + // and the slice. + assert(parent->blob_->refs >= 2); return scope.Close(string); } -static Handle Utf8Slice(const Arguments &args) { - HandleScope scope; +Handle Buffer::Utf8Slice(const Arguments &args) { + HandleScope scope; + Buffer *parent = ObjectWrap::Unwrap(args.This()); SLICE_ARGS(args[0], args[1]) - - struct buffer *parent = BufferUnwrap(args.This()); - size_t start_abs = buffer_abs_off(parent, start); - size_t end_abs = buffer_abs_off(parent, end); - assert(start_abs <= end_abs); - - struct buffer *root = buffer_root(parent); - - Local string = - String::New(reinterpret_cast(&root->bytes + start_abs), - end_abs - start_abs); + const char *data = reinterpret_cast(parent->data_ + start); + Local string = String::New(data, end - start); return scope.Close(string); } -static Handle Slice(const Arguments &args) { - HandleScope scope; +Handle Buffer::Slice(const Arguments &args) { + HandleScope scope; Local argv[3] = { args.This(), args[0], args[1] }; - Local slice = constructor_template->GetFunction()->NewInstance(3, argv); - return scope.Close(slice); } -// var charsWritten = buffer.utf8Write(string, offset, length); -static Handle Utf8Write(const Arguments &args) { +// var charsWritten = buffer.utf8Write(string, offset); +Handle Buffer::Utf8Write(const Arguments &args) { HandleScope scope; - - struct buffer *buffer = BufferUnwrap(args.This()); + Buffer *buffer = ObjectWrap::Unwrap(args.This()); if (!args[0]->IsString()) { return ThrowException(Exception::TypeError(String::New( @@ -268,30 +212,28 @@ static Handle Utf8Write(const Arguments &args) { size_t offset = args[1]->Int32Value(); - char *p = buffer_p(buffer, offset); - if (buffer_p(buffer, offset) == NULL) { + if (offset >= buffer->length_) { return ThrowException(Exception::TypeError(String::New( "Offset is out of bounds"))); } - size_t toWrite = args[2]->Int32Value(); + const char *p = buffer->data_ + offset; - if (buffer_remaining(buffer, offset) < toWrite) { + if (s->Length() + offset > buffer->length_) { return ThrowException(Exception::TypeError(String::New( - "Length is out of bounds"))); + "Not enough space in Buffer for string"))); } - int written = s->WriteUtf8(p, toWrite); - + int written = s->WriteUtf8((char*)p); return scope.Close(Integer::New(written)); } -// var charsWritten = buffer.asciiWrite(string, offset, length); -static Handle AsciiWrite(const Arguments &args) { +// var charsWritten = buffer.asciiWrite(string, offset); +Handle Buffer::AsciiWrite(const Arguments &args) { HandleScope scope; - struct buffer *buffer = BufferUnwrap(args.This()); + Buffer *buffer = ObjectWrap::Unwrap(args.This()); if (!args[0]->IsString()) { return ThrowException(Exception::TypeError(String::New( @@ -302,30 +244,25 @@ static Handle AsciiWrite(const Arguments &args) { size_t offset = args[1]->Int32Value(); - char *p = buffer_p(buffer, offset); - if (buffer_p(buffer, offset) == NULL) { + if (offset >= buffer->length_) { return ThrowException(Exception::TypeError(String::New( "Offset is out of bounds"))); } - size_t toWrite = args[2]->Int32Value(); + const char *p = buffer->data_ + offset; - if (buffer_remaining(buffer, offset) < toWrite) { + if (s->Length() + offset > buffer->length_) { return ThrowException(Exception::TypeError(String::New( - "Length is out of bounds"))); + "Not enough space in Buffer for string"))); } - // TODO Expose the second argument of WriteAscii? - // Could avoid doing slices when the string doesn't fit in a buffer. V8 - // slice() does copy the string, so exposing that argument would help. - - int written = s->WriteAscii(p, 0, toWrite); - + int written = s->WriteAscii((char*)p); return scope.Close(Integer::New(written)); } -static Handle Utf8Length(const Arguments &args) { +// var nbytes = Buffer.utf8Length("string") +Handle Buffer::Utf8Length(const Arguments &args) { HandleScope scope; if (!args[0]->IsString()) { return ThrowException(Exception::TypeError(String::New( @@ -336,29 +273,37 @@ static Handle Utf8Length(const Arguments &args) { } -void InitBuffer(Handle target) { +bool Buffer::HasInstance(Handle val) { + if (!val->IsObject()) return false; + Local obj = val->ToObject(); + return constructor_template->HasInstance(obj); +} + + +void Buffer::Initialize(Handle target) { HandleScope scope; length_symbol = Persistent::New(String::NewSymbol("length")); - Local t = FunctionTemplate::New(Constructor); + Local t = FunctionTemplate::New(Buffer::New); constructor_template = Persistent::New(t); constructor_template->InstanceTemplate()->SetInternalFieldCount(1); constructor_template->SetClassName(String::NewSymbol("Buffer")); // copy free - NODE_SET_PROTOTYPE_METHOD(constructor_template, "asciiSlice", AsciiSlice); - NODE_SET_PROTOTYPE_METHOD(constructor_template, "slice", Slice); + NODE_SET_PROTOTYPE_METHOD(constructor_template, "asciiSlice", Buffer::AsciiSlice); + NODE_SET_PROTOTYPE_METHOD(constructor_template, "slice", Buffer::Slice); // TODO NODE_SET_PROTOTYPE_METHOD(t, "utf16Slice", Utf16Slice); - // copy - NODE_SET_PROTOTYPE_METHOD(constructor_template, "utf8Slice", Utf8Slice); + // copy + NODE_SET_PROTOTYPE_METHOD(constructor_template, "utf8Slice", Buffer::Utf8Slice); - NODE_SET_PROTOTYPE_METHOD(constructor_template, "utf8Write", Utf8Write); - NODE_SET_PROTOTYPE_METHOD(constructor_template, "asciiWrite", AsciiWrite); + NODE_SET_PROTOTYPE_METHOD(constructor_template, "utf8Write", Buffer::Utf8Write); + NODE_SET_PROTOTYPE_METHOD(constructor_template, "asciiWrite", Buffer::AsciiWrite); - NODE_SET_METHOD(constructor_template->GetFunction(), "utf8Length", Utf8Length); + NODE_SET_METHOD(constructor_template->GetFunction(), "utf8Length", Buffer::Utf8Length); target->Set(String::NewSymbol("Buffer"), constructor_template->GetFunction()); } + } // namespace node diff --git a/src/node_buffer.h b/src/node_buffer.h index 122d589d85..d6b83fe00b 100644 --- a/src/node_buffer.h +++ b/src/node_buffer.h @@ -1,19 +1,19 @@ -#ifndef NODE_BUFFER -#define NODE_BUFFER +#ifndef NODE_BUFFER_H_ +#define NODE_BUFFER_H_ +#include +#include #include namespace node { -#define MIN(a,b) ((a) < (b) ? (a) : (b)) - /* A buffer is a chunk of memory stored outside the V8 heap, mirrored by an * object in javascript. The object is not totally opaque, one can access * individual bytes with [] and slice it into substrings or sub-buffers * without copying memory. * * // return an ascii encoded string - no memory iscopied - * buffer.asciiSlide(0, 3) + * buffer.asciiSlide(0, 3) * * // returns another buffer - no memory is copied * buffer.slice(0, 3) @@ -25,40 +25,42 @@ namespace node { * are GCed. */ -struct buffer { - v8::Persistent handle; // both - bool weak; // both - struct buffer *root; // both (NULL for root) - size_t offset; // both (0 for root) - size_t length; // both - unsigned int refs; // root only - char bytes[1]; // root only -}; -void InitBuffer(v8::Handle target); +struct Blob_; + +class Buffer : public ObjectWrap { + public: + static void Initialize(v8::Handle target); + static bool HasInstance(v8::Handle val); -struct buffer* BufferUnwrap(v8::Handle val); -bool IsBuffer(v8::Handle val); + const char* data() const { return data_; } + size_t length() const { return length_; } + struct Blob_* blob() const { return blob_; } -static inline struct buffer * buffer_root(struct buffer *buffer) { - return buffer->root ? buffer->root : buffer; -} + protected: + static v8::Persistent constructor_template; + static v8::Handle New(const v8::Arguments &args); + static v8::Handle Slice(const v8::Arguments &args); + static v8::Handle AsciiSlice(const v8::Arguments &args); + static v8::Handle Utf8Slice(const v8::Arguments &args); + static v8::Handle AsciiWrite(const v8::Arguments &args); + static v8::Handle Utf8Write(const v8::Arguments &args); + static v8::Handle Utf8Length(const v8::Arguments &args); -static inline char * buffer_p(struct buffer *buffer, size_t off) { - struct buffer *root = buffer_root(buffer); - if (buffer->offset + off >= root->length) return NULL; - return reinterpret_cast(&(root->bytes) + buffer->offset + off); -} + int AsciiWrite(char *string, int offset, int length); + int Utf8Write(char *string, int offset, int length); -static inline size_t buffer_remaining(struct buffer *buffer, size_t off) { - struct buffer *root = buffer_root(buffer); - char *end = reinterpret_cast(&(root->bytes) + root->length); - return end - buffer_p(buffer, off); -} + private: + Buffer(size_t length); + Buffer(Buffer *parent, size_t start, size_t end); + ~Buffer(); + + const char *data_; + size_t length_; + struct Blob_ *blob_; +}; -void buffer_ref(struct buffer *buffer); -void buffer_unref(struct buffer *buffer); } // namespace node buffer -#endif // NODE_BUFFER +#endif // NODE_BUFFER_H_ diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 9ec5005361..2236b47359 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -65,41 +65,35 @@ static Persistent version_minor_sym; static Persistent should_keep_alive_sym; // Callback prototype for http_cb -#define DEFINE_HTTP_CB(name) \ - static int name(http_parser *p) { \ - Parser *parser = static_cast(p->data); \ - \ - HandleScope scope; \ - \ - Local cb_value = parser->handle_->Get(name##_sym); \ - if (!cb_value->IsFunction()) return 0; \ - Local cb = Local::Cast(cb_value); \ - Local ret = cb->Call(parser->handle_, 0, NULL); \ - return ret.IsEmpty() ? -1 : 0; \ +#define DEFINE_HTTP_CB(name) \ + static int name(http_parser *p) { \ + Parser *parser = static_cast(p->data); \ + \ + HandleScope scope; \ + \ + Local cb_value = parser->handle_->Get(name##_sym); \ + if (!cb_value->IsFunction()) return 0; \ + Local cb = Local::Cast(cb_value); \ + Local ret = cb->Call(parser->handle_, 0, NULL); \ + return ret.IsEmpty() ? -1 : 0; \ } // Callback prototype for http_data_cb -#define DEFINE_HTTP_DATA_CB(name) \ - static int name(http_parser *p, const char *at, size_t length) { \ - Parser *parser = static_cast(p->data); \ - \ - HandleScope scope; \ - \ - assert(parser->buffer_); \ - struct buffer * root = buffer_root(parser->buffer_); \ - char * base = buffer_p(root, 0); \ - \ - Local cb_value = parser->handle_->Get(name##_sym); \ - if (!cb_value->IsFunction()) return 0; \ - Local cb = Local::Cast(cb_value); \ - \ - Local argv[3] = { Local::New(root->handle) \ - , Integer::New(at - base) \ - , Integer::New(length) \ - }; \ - Local ret = cb->Call(parser->handle_, 3, argv); \ - assert(parser->buffer_); \ - return ret.IsEmpty() ? -1 : 0; \ +#define DEFINE_HTTP_DATA_CB(name) \ + static int name(http_parser *p, const char *at, size_t length) { \ + Parser *parser = static_cast(p->data); \ + HandleScope scope; \ + assert(parser->buffer_); \ + Local cb_value = parser->handle_->Get(name##_sym); \ + if (!cb_value->IsFunction()) return 0; \ + Local cb = Local::Cast(cb_value); \ + Local argv[3] = { Local::New(parser->buffer_->handle_) \ + , Integer::New(at - parser->buffer_->data()) \ + , Integer::New(length) \ + }; \ + Local ret = cb->Call(parser->handle_, 3, argv); \ + assert(parser->buffer_); \ + return ret.IsEmpty() ? -1 : 0; \ } @@ -218,21 +212,21 @@ class Parser : public ObjectWrap { String::New("Already parsing a buffer"))); } - if (!IsBuffer(args[0])) { + if (!Buffer::HasInstance(args[0])) { return ThrowException(Exception::TypeError( String::New("Argument should be a buffer"))); } - struct buffer * buffer = BufferUnwrap(args[0]); + Buffer * buffer = ObjectWrap::Unwrap(args[0]->ToObject()); size_t off = args[1]->Int32Value(); - if (buffer_p(buffer, off) == NULL) { + if (off >= buffer->length()) { return ThrowException(Exception::Error( String::New("Offset is out of bounds"))); } size_t len = args[2]->Int32Value(); - if (buffer_remaining(buffer, off) < len) { + if (off+len > buffer->length()) { return ThrowException(Exception::Error( String::New("Length is extends beyond buffer"))); } @@ -242,8 +236,8 @@ class Parser : public ObjectWrap { // Assign 'buffer_' while we parse. The callbacks will access that varible. parser->buffer_ = buffer; - size_t nparsed = - http_parser_execute(&(parser->parser_), buffer_p(buffer, off), len); + size_t nparsed = + http_parser_execute(&parser->parser_, buffer->data()+off, len); // Unassign the 'buffer_' variable assert(parser->buffer_); @@ -318,7 +312,7 @@ class Parser : public ObjectWrap { parser_.data = this; } - struct buffer * buffer_; // The buffer currently being parsed. + Buffer * buffer_; // The buffer currently being parsed. http_parser parser_; }; diff --git a/src/node_io_watcher.cc b/src/node_io_watcher.cc index baf46411bc..4a2d9378d9 100644 --- a/src/node_io_watcher.cc +++ b/src/node_io_watcher.cc @@ -11,7 +11,7 @@ namespace node { using namespace v8; Persistent IOWatcher::constructor_template; -static Persistent callback_symbol; +Persistent callback_symbol; void IOWatcher::Initialize(Handle target) { HandleScope scope; @@ -34,7 +34,6 @@ void IOWatcher::Initialize(Handle target) { void IOWatcher::Callback(EV_P_ ev_io *w, int revents) { IOWatcher *io = static_cast(w->data); assert(w == &io->watcher_); - assert(!(revents & EV_ERROR)); HandleScope scope; Local callback_v = io->handle_->Get(callback_symbol); @@ -51,9 +50,7 @@ void IOWatcher::Callback(EV_P_ ev_io *w, int revents) { argv[0] = Local::New(revents & EV_READ ? True() : False()); argv[1] = Local::New(revents & EV_WRITE ? True() : False()); - io->Ref(); callback->Call(io->handle_, 2, argv); - io->Unref(); if (try_catch.HasCaught()) { FatalException(try_catch); @@ -62,8 +59,9 @@ void IOWatcher::Callback(EV_P_ ev_io *w, int revents) { // -// var io = new process.IOWatcher(); -// io.callback = function (readable, writable) { ... }; +// var io = new process.IOWatcher(function (readable, writable) { +// +// }); // io.set(fd, true, false); // io.start(); // @@ -71,24 +69,34 @@ Handle IOWatcher::New(const Arguments& args) { HandleScope scope; IOWatcher *s = new IOWatcher(); - - s->Wrap(args.This()); return args.This(); } +Handle IOWatcher::Start(const Arguments& args) { + HandleScope scope; + + IOWatcher *io = ObjectWrap::Unwrap(args.Holder()); + + ev_io_start(EV_DEFAULT_UC_ &io->watcher_); + + io->Ref(); + + return Undefined(); +} + Handle IOWatcher::Set(const Arguments& args) { HandleScope scope; + IOWatcher *io = ObjectWrap::Unwrap(args.Holder()); + if (!args[0]->IsInt32()) { return ThrowException(Exception::TypeError( String::New("First arg should be a file descriptor."))); } - IOWatcher *io = ObjectWrap::Unwrap(args.This()); - int fd = args[0]->Int32Value(); if (!args[1]->IsBoolean()) { @@ -112,38 +120,19 @@ Handle IOWatcher::Set(const Arguments& args) { return Undefined(); } - -Handle IOWatcher::Start(const Arguments& args) { - HandleScope scope; - IOWatcher *io = ObjectWrap::Unwrap(args.This()); - io->Start(); - return Undefined(); -} - - Handle IOWatcher::Stop(const Arguments& args) { HandleScope scope; - IOWatcher *io = ObjectWrap::Unwrap(args.This()); + IOWatcher *io = ObjectWrap::Unwrap(args.Holder()); io->Stop(); return Undefined(); } -void IOWatcher::Start () { - if (!ev_is_active(&watcher_)) { - ev_io_start(EV_DEFAULT_UC_ &watcher_); - Ref(); - } - assert(ev_is_active(&watcher_)); -} - - void IOWatcher::Stop () { - if (ev_is_active(&watcher_)) { + if (watcher_.active) { ev_io_stop(EV_DEFAULT_UC_ &watcher_); Unref(); } - assert(!ev_is_active(&watcher_)); } diff --git a/src/node_net2.cc b/src/node_net2.cc index 47e8da3cf0..241469873e 100644 --- a/src/node_net2.cc +++ b/src/node_net2.cc @@ -488,28 +488,26 @@ static Handle Read(const Arguments& args) { FD_ARG(args[0]) - if (!IsBuffer(args[1])) { + if (!Buffer::HasInstance(args[1])) { return ThrowException(Exception::TypeError( String::New("Second argument should be a buffer"))); } - struct buffer * buffer = BufferUnwrap(args[1]); + Buffer * buffer = ObjectWrap::Unwrap(args[1]->ToObject()); size_t off = args[2]->Int32Value(); - if (buffer_p(buffer, off) == NULL) { + if (off >= buffer->length()) { return ThrowException(Exception::Error( String::New("Offset is out of bounds"))); } size_t len = args[3]->Int32Value(); - if (buffer_remaining(buffer, off) < len) { + if (off + len > buffer->length()) { return ThrowException(Exception::Error( String::New("Length is extends beyond buffer"))); } - ssize_t bytes_read = read(fd, - buffer_p(buffer, off), - buffer_remaining(buffer, off)); + ssize_t bytes_read = read(fd, (char*)buffer->data() + off, len); if (bytes_read < 0) { if (errno == EAGAIN || errno == EINTR) return Null(); @@ -533,21 +531,21 @@ static Handle RecvMsg(const Arguments& args) { FD_ARG(args[0]) - if (!IsBuffer(args[1])) { + if (!Buffer::HasInstance(args[1])) { return ThrowException(Exception::TypeError( String::New("Second argument should be a buffer"))); } - struct buffer * buffer = BufferUnwrap(args[1]); + Buffer * buffer = ObjectWrap::Unwrap(args[1]->ToObject()); size_t off = args[2]->Int32Value(); - if (buffer_p(buffer, off) == NULL) { + if (off >= buffer->length()) { return ThrowException(Exception::Error( String::New("Offset is out of bounds"))); } size_t len = args[3]->Int32Value(); - if (buffer_remaining(buffer, off) < len) { + if (off + len > buffer->length()) { return ThrowException(Exception::Error( String::New("Length is extends beyond buffer"))); } @@ -555,7 +553,7 @@ static Handle RecvMsg(const Arguments& args) { int received_fd; struct iovec iov[1]; - iov[0].iov_base = buffer_p(buffer, off); + iov[0].iov_base = (char*)buffer->data() + off; iov[0].iov_len = len; struct msghdr msg; @@ -606,28 +604,26 @@ static Handle Write(const Arguments& args) { FD_ARG(args[0]) - if (!IsBuffer(args[1])) { + if (!Buffer::HasInstance(args[1])) { return ThrowException(Exception::TypeError( String::New("Second argument should be a buffer"))); } - struct buffer * buffer = BufferUnwrap(args[1]); + Buffer * buffer = ObjectWrap::Unwrap(args[1]->ToObject()); size_t off = args[2]->Int32Value(); - char *p = buffer_p(buffer, off); - if (p == NULL) { + if (off >= buffer->length()) { return ThrowException(Exception::Error( String::New("Offset is out of bounds"))); } size_t len = args[3]->Int32Value(); - size_t remaining = buffer_remaining(buffer, off); - if (remaining < len) { + if (off + len > buffer->length()) { return ThrowException(Exception::Error( String::New("Length is extends beyond buffer"))); } - ssize_t written = write(fd, p, len); + ssize_t written = write(fd, (char*)buffer->data() + off, len); if (written < 0) { if (errno == EAGAIN || errno == EINTR) return Null(); @@ -662,9 +658,9 @@ static Handle SendFD(const Arguments& args) { struct iovec iov[1]; char control_msg[CMSG_SPACE(sizeof(fd_to_send))]; struct cmsghdr *cmsg; - char *dummy = "d"; // Need to send at least a byte of data in the message + static char dummy = 'd'; // Need to send at least a byte of data in the message - iov[0].iov_base = dummy; + iov[0].iov_base = &dummy; iov[0].iov_len = 1; msg.msg_iov = iov; msg.msg_iovlen = 1; diff --git a/src/node_object_wrap.h b/src/node_object_wrap.h index 6656664bdf..1d3fafa48b 100644 --- a/src/node_object_wrap.h +++ b/src/node_object_wrap.h @@ -21,7 +21,8 @@ class ObjectWrap { } } - protected: + v8::Persistent handle_; // ro + template static inline T* Unwrap (v8::Handle handle) { @@ -31,6 +32,7 @@ class ObjectWrap { handle->GetInternalField(0))->Value()); } + protected: inline void Wrap (v8::Handle handle) { assert(handle_.IsEmpty()); @@ -72,7 +74,6 @@ class ObjectWrap { if (refs_ == 0 && handle_.IsNearDeath()) delete this; } - v8::Persistent handle_; // ro int refs_; // ro private: diff --git a/test/mjsunit/test-buffer.js b/test/mjsunit/test-buffer.js index cfeafa1fce..ebdbf67379 100644 --- a/test/mjsunit/test-buffer.js +++ b/test/mjsunit/test-buffer.js @@ -16,16 +16,28 @@ for (var i = 0; i < 1024; i++) { assert.equal(i % 256, b[i]); } -for (var j = 0; j < 10000; j++) { - var asciiString = "hello world"; +var asciiString = "hello world"; +var offset = 100; +for (var j = 0; j < 50000; j++) { for (var i = 0; i < asciiString.length; i++) { b[i] = asciiString.charCodeAt(i); } - var asciiSlice = b.asciiSlice(0, asciiString.length); + assert.equal(asciiString, asciiSlice); + var written = b.asciiWrite(asciiString, offset); + assert.equal(asciiString.length, written); + var asciiSlice = b.asciiSlice(offset, offset+asciiString.length); assert.equal(asciiString, asciiSlice); + + var sliceA = b.slice(offset, offset+asciiString.length); + var sliceB = b.slice(offset, offset+asciiString.length); + for (var i = 0; i < asciiString.length; i++) { + assert.equal(sliceA[i], sliceB[i]); + } + + // TODO utf8 slice tests } @@ -36,3 +48,6 @@ for (var j = 0; j < 10000; j++) { assert.equal(b[100+i], slice[i]); } } + + + diff --git a/wscript b/wscript index bd2425035c..c077d8aff8 100644 --- a/wscript +++ b/wscript @@ -345,8 +345,8 @@ def build(bld): node.source = """ src/node.cc src/node_buffer.cc - src/node_net2.cc src/node_http_parser.cc + src/node_net2.cc src/node_io_watcher.cc src/node_child_process.cc src/node_constants.cc