diff --git a/doc/api.markdown b/doc/api.markdown index 9999148799..488035e37b 100644 --- a/doc/api.markdown +++ b/doc/api.markdown @@ -1392,11 +1392,15 @@ or 'a+'. The callback gets two arguments `(err, fd)`. Synchronous open(2). -### fs.write(fd, data, position, encoding, callback) +### fs.write(fd, buffer, offset, length, position, callback) -Write data to the file specified by `fd`. `position` refers to the offset -from the beginning of the file where this data should be written. If -`position` is `null`, the data will be written at the current position. +Write `buffer` to the file specified by `fd`. + +`offset` and `length` determine the part of the buffer to be written. + +`position` refers to the offset from the beginning of the file where this data +should be written. If `position` is `null`, the data will be written at the +current position. See pwrite(2). The callback will be given two arguments `(err, written)` where `written` diff --git a/lib/fs.js b/lib/fs.js index ec68c02e9d..65469287bc 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -153,14 +153,31 @@ fs.readSync = function (fd, length, position, encoding) { return binding.read(fd, length, position, encoding); }; -fs.write = function (fd, data, position, encoding, callback) { - encoding = encoding || "binary"; - binding.write(fd, data, position, encoding, callback || noop); +fs.write = function (fd, buffer, offset, length, position, callback) { + if (!(buffer instanceof Buffer)) { + // legacy string interface (fd, data, position, encoding, callback) + callback = arguments[4]; + position = arguments[2]; + + buffer = new Buffer(''+arguments[1], arguments[3]); + offset = 0; + length = buffer.length; + } + + binding.write(fd, buffer, offset, length, position, callback || noop); }; -fs.writeSync = function (fd, data, position, encoding) { - encoding = encoding || "binary"; - return binding.write(fd, data, position, encoding); +fs.writeSync = function (fd, buffer, offset, length, position) { + if (!(buffer instanceof Buffer)) { + // legacy string interface (fd, data, position, encoding) + position = arguments[2]; + + buffer = new Buffer(''+arguments[1], arguments[3]); + offset = 0; + length = buffer.length; + } + + binding.write(fd, buffer, offset, length, position); }; fs.rename = function (oldPath, newPath, callback) { diff --git a/src/node_file.cc b/src/node_file.cc index f36b79e6e6..cb974c2347 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -150,11 +150,6 @@ static int After(eio_req *req) { } } - if (req->type == EIO_WRITE && req->int3 == 1) { - assert(req->ptr2); - delete [] reinterpret_cast(req->ptr2); - } - TryCatch try_catch; (*callback)->Call(Context::GetCurrent()->Global(), argc, argv); @@ -536,14 +531,6 @@ static Handle Open(const Arguments& args) { // 3 length how much to write // 4 position if integer, position to write at in the file. // if null, write from the current position -// -// - OR - -// -// 0 fd integer. file descriptor -// 1 string the data to write -// 2 position if integer, position to write at in the file. -// if null, write from the current position -// 3 encoding static Handle Write(const Arguments& args) { HandleScope scope; @@ -553,98 +540,34 @@ static Handle Write(const Arguments& args) { int fd = args[0]->Int32Value(); - off_t pos; - ssize_t len; - char * buf; - ssize_t written; - - Local cb; - bool legacy; - - if (Buffer::HasInstance(args[1])) { - legacy = false; - // buffer - // - // 0 fd integer. file descriptor - // 1 buffer the data to write - // 2 offset where in the buffer to start from - // 3 length how much to write - // 4 position if integer, position to write at in the file. - // if null, write from the current position - - Buffer * buffer = ObjectWrap::Unwrap(args[1]->ToObject()); - - size_t off = args[2]->Int32Value(); - if (off >= buffer->length()) { - return ThrowException(Exception::Error( - String::New("Offset is out of bounds"))); - } - - len = args[3]->Int32Value(); - if (off + len > buffer->length()) { - return ThrowException(Exception::Error( - String::New("Length is extends beyond buffer"))); - } - - pos = GET_OFFSET(args[4]); - - buf = (char*)buffer->data() + off; - - cb = args[5]; + if (!Buffer::HasInstance(args[1])) { + return ThrowException(Exception::Error( + String::New("Second argument needs to be a buffer"))); + } - } else { - legacy = true; - // legacy interface.. args[1] is a string + Buffer * buffer = ObjectWrap::Unwrap(args[1]->ToObject()); - pos = GET_OFFSET(args[2]); + size_t off = args[2]->Int32Value(); + if (off >= buffer->length()) { + return ThrowException(Exception::Error( + String::New("Offset is out of bounds"))); + } - enum encoding enc = ParseEncoding(args[3]); + ssize_t len = args[3]->Int32Value(); + if (off + len > buffer->length()) { + return ThrowException(Exception::Error( + String::New("Length is extends beyond buffer"))); + } - len = DecodeBytes(args[1], enc); - if (len < 0) { - Local exception = Exception::TypeError(String::New("Bad argument")); - return ThrowException(exception); - } + off_t pos = GET_OFFSET(args[4]); - buf = new char[len]; - written = DecodeWrite(buf, len, args[1], enc); - assert(written == len); - - cb = args[4]; - } + char * buf = (char*)buffer->data() + off; + Local cb = args[5]; if (cb->IsFunction()) { - // WARNING: HACK AHEAD, PROCEED WITH CAUTION - // Normally here I would do - // ASYNC_CALL(write, cb, fd, buf, len, pos) - // however, I'm trying to support a legacy interface; where in the - // String version a chunk of memory is allocated for the string to be - // decoded into. In the After() function it is freed. - // - // However in the Buffer version, we increase the reference count - // to the Buffer while we're in the thread pool. - // - // We have to let After() know which version is being done so it can - // know if it needs to free 'buf' or not. We do that by using - // req->int3. - // - // req->int3 == 1 legacy, String version. Need to free `buf`. - // req->int3 == 0 Buffer version. Don't do anything. - // - eio_req *req = eio_write(fd, buf, len, pos, - EIO_PRI_DEFAULT, - After, - cb_persist(cb)); - assert(req); - req->int3 = legacy ? 1 : 0; - ev_ref(EV_DEFAULT_UC); - return Undefined(); - + ASYNC_CALL(write, cb, fd, buf, len, pos) } else { - written = pos < 0 ? write(fd, buf, len) : pwrite(fd, buf, len, pos); - if (legacy) { - delete [] reinterpret_cast(buf); - } + ssize_t written = pos < 0 ? write(fd, buf, len) : pwrite(fd, buf, len, pos); if (written < 0) return ThrowException(ErrnoException(errno)); return scope.Close(Integer::New(written)); } diff --git a/test/simple/test-fs-write-buffer.js b/test/simple/test-fs-write-buffer.js new file mode 100644 index 0000000000..ca96b2312d --- /dev/null +++ b/test/simple/test-fs-write-buffer.js @@ -0,0 +1,32 @@ +require('../common'); +var path = require('path') + , Buffer = require('buffer').Buffer + , fs = require('fs') + , fn = path.join(fixturesDir, 'write.txt') + , expected = new Buffer('hello') + , openCalled = 0 + , writeCalled = 0; + + +fs.open(fn, 'w', 0644, function (err, fd) { + openCalled++; + if (err) throw err; + + fs.write(fd, expected, 0, expected.length, null, function(err, written) { + writeCalled++; + if (err) throw err; + + assert.equal(expected.length, written); + fs.closeSync(fd); + + var found = fs.readFileSync(fn); + assert.deepEqual(found, expected.toString()); + fs.unlinkSync(fn); + }); +}); + +process.addListener("exit", function () { + assert.equal(openCalled, 1); + assert.equal(writeCalled, 1); +}); + diff --git a/test/simple/test-fs-write-sync.js b/test/simple/test-fs-write-sync.js new file mode 100644 index 0000000000..2e30cc704e --- /dev/null +++ b/test/simple/test-fs-write-sync.js @@ -0,0 +1,12 @@ +require('../common'); +var path = require('path') + , Buffer = require('buffer').Buffer + , fs = require('fs') + , fn = path.join(fixturesDir, 'write.txt'); + +var fd = fs.openSync(fn, 'w'); +fs.writeSync(fd, 'foo'); +fs.writeSync(fd, new Buffer('bar'), 0, 3); +fs.closeSync(fd); + +assert.equal(fs.readFileSync(fn), 'foobar'); \ No newline at end of file