diff --git a/doc/api.markdown b/doc/api.markdown index 488035e37b..6afc4661af 100644 --- a/doc/api.markdown +++ b/doc/api.markdown @@ -1410,20 +1410,24 @@ specifies how many _bytes_ were written. Synchronous version of `fs.write()`. Returns the number of bytes written. -### fs.read(fd, length, position, encoding, callback) +### fs.read(fd, buffer, offset, length, position, callback) Read data from the file specified by `fd`. +`buffer` is the buffer that the data will be written to. + +`offset` is offset within the buffer where writing will start. + `length` is an integer specifying the number of bytes to read. `position` is an integer specifying where to begin reading from in the file. +If `position` is `null`, data will be read from the current file position. -The callback is given three arguments, `(err, data, bytesRead)` where `data` -is a string--what was read--and `bytesRead` is the number of bytes read. +The callback is given the two arguments, `(err, bytesRead)`. -### fs.readSync(fd, length, position, encoding) +### fs.readSync(fd, buffer, offset, length, position) -Synchronous version of `fs.read`. Returns an array `[data, bytesRead]`. +Synchronous version of `fs.read`. Returns the number of `bytesRead`. ### fs.readFile(filename, [encoding,] callback) diff --git a/lib/fs.js b/lib/fs.js index ec5a113eb8..fc0843b730 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -92,7 +92,7 @@ fs.readFileSync = function (path, encoding) { var pos = null; // leave null to allow reads on unseekable devices var r; - while ((r = binding.read(fd, 4*1024, pos, encoding)) && r[0]) { + while ((r = fs.read(fd, 4*1024, pos, encoding)) && r[0]) { content += r[0]; pos += r[1] } @@ -143,14 +143,54 @@ fs.openSync = function (path, flags, mode) { return binding.open(path, stringToFlags(flags), mode); }; -fs.read = function (fd, length, position, encoding, callback) { - encoding = encoding || "binary"; - binding.read(fd, length, position, encoding, callback || noop); +fs.read = function (fd, buffer, offset, length, position, callback) { + if (!(buffer instanceof Buffer)) { + // legacy string interface (fd, length, position, encoding, callback) + var cb = arguments[4]; + encoding = arguments[3]; + position = arguments[2]; + length = arguments[1]; + buffer = new Buffer(length); + offset = 0; + + callback = function(err, bytesRead) { + if (!cb) { + return; + } + + var str = (bytesRead > 0) + ? buffer.toString(encoding, 0, bytesRead) + : ''; + + (cb)(err, str, bytesRead); + }; + } + + binding.read(fd, buffer, offset, length, position, callback || noop); }; -fs.readSync = function (fd, length, position, encoding) { - encoding = encoding || "binary"; - return binding.read(fd, length, position, encoding); +fs.readSync = function (fd, buffer, offset, length, position) { + var legacy = false; + if (!(buffer instanceof Buffer)) { + // legacy string interface (fd, length, position, encoding, callback) + legacy = true; + encoding = arguments[3]; + position = arguments[2]; + length = arguments[1]; + buffer = new Buffer(length); + + offset = 0; + } + + var r = binding.read(fd, buffer, offset, length, position); + if (!legacy) { + return r; + } + + var str = (r > 0) + ? buffer.toString(encoding, 0, r) + : ''; + return [str, r]; }; fs.write = function (fd, buffer, offset, length, position, callback) { diff --git a/src/node_file.cc b/src/node_file.cc index 6e9ef1a4d6..49ba894dcf 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -106,18 +106,9 @@ static int After(eio_req *req) { case EIO_READ: { - if (req->int3) { - // legacy interface - Local obj = Local::New(*callback); - Local enc_val = obj->GetHiddenValue(encoding_symbol); - argv[1] = Encode(req->ptr2, req->result, ParseEncoding(enc_val)); - argv[2] = Integer::New(req->result); - argc = 3; - } else { - // Buffer interface - argv[1] = Integer::New(req->result); - argc = 2; - } + // Buffer interface + argv[1] = Integer::New(req->result); + argc = 2; break; } @@ -584,16 +575,6 @@ static Handle Write(const Arguments& args) { * 3 length integer. length to read * 4 position file position - null for current position * - * - OR - - * - * [string, bytesRead] = fs.read(fd, length, position, encoding) - * - * 0 fd integer. file descriptor - * 1 length integer. length to read - * 2 position if integer, position to read from in the file. - * if null, read from the current position - * 3 encoding - * */ static Handle Read(const Arguments& args) { HandleScope scope; @@ -605,105 +586,47 @@ static Handle Read(const Arguments& args) { int fd = args[0]->Int32Value(); Local cb; - bool legacy; size_t len; off_t pos; - enum encoding encoding; char * buf = NULL; - if (Buffer::HasInstance(args[1])) { - legacy = false; - // 0 fd integer. file descriptor - // 1 buffer instance of Buffer - // 2 offset integer. offset to start reading into inside buffer - // 3 length integer. length to read - // 4 position file position - null for 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]; - - } else { - legacy = true; - // 0 fd integer. file descriptor - // 1 length integer. length to read - // 2 position if integer, position to read from in the file. - // if null, read from the current position - // 3 encoding - len = args[1]->IntegerValue(); - pos = GET_OFFSET(args[2]); - encoding = ParseEncoding(args[3]); + if (!Buffer::HasInstance(args[1])) { + return ThrowException(Exception::Error( + String::New("Second argument needs to be a buffer"))); + } - buf = NULL; // libeio will allocate and free it. + Buffer * buffer = ObjectWrap::Unwrap(args[1]->ToObject()); - cb = args[4]; + 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"))); + } - if (cb->IsFunction()) { - // WARNING: HACK AGAIN, PROCEED WITH CAUTION - // Normally here I would do - // ASYNC_CALL(read, args[4], fd, NULL, len, offset) - // but I'm trying to support a legacy interface where it's acceptable to - // return a string in the callback. As well as a new Buffer interface - // which reads data into a user supplied buffer. - - // Set the encoding on the callback - if (legacy) { - Local obj = cb->ToObject(); - obj->SetHiddenValue(encoding_symbol, args[3]); - } - - if (legacy) assert(buf == NULL); + pos = GET_OFFSET(args[4]); - - eio_req *req = eio_read(fd, buf, len, pos, - EIO_PRI_DEFAULT, - After, - cb_persist(cb)); - assert(req); + buf = (char*)buffer->data() + off; - req->int3 = legacy ? 1 : 0; - ev_ref(EV_DEFAULT_UC); - return Undefined(); + cb = args[5]; + if (cb->IsFunction()) { + ASYNC_CALL(read, cb, fd, buf, len, pos); } else { // SYNC ssize_t ret; - if (legacy) { -#define READ_BUF_LEN (16*1024) - char buf2[READ_BUF_LEN]; - ret = pos < 0 ? read(fd, buf2, MIN(len, READ_BUF_LEN)) - : pread(fd, buf2, MIN(len, READ_BUF_LEN), pos); - if (ret < 0) return ThrowException(ErrnoException(errno)); - Local a = Array::New(2); - a->Set(Integer::New(0), Encode(buf2, ret, encoding)); - a->Set(Integer::New(1), Integer::New(ret)); - return scope.Close(a); - } else { - ret = pos < 0 ? read(fd, buf, len) : pread(fd, buf, len, pos); - if (ret < 0) return ThrowException(ErrnoException(errno)); - Local bytesRead = Integer::New(ret); - return scope.Close(bytesRead); - } + ret = pos < 0 ? read(fd, buf, len) : pread(fd, buf, len, pos); + if (ret < 0) return ThrowException(ErrnoException(errno)); + Local bytesRead = Integer::New(ret); + return scope.Close(bytesRead); } } diff --git a/test/simple/test-fs-read-buffer.js b/test/simple/test-fs-read-buffer.js new file mode 100644 index 0000000000..5a18177a4d --- /dev/null +++ b/test/simple/test-fs-read-buffer.js @@ -0,0 +1,25 @@ +require('../common'); +var path = require('path'), + Buffer = require('buffer').Buffer, + fs = require('fs'), + filepath = path.join(fixturesDir, 'x.txt'), + fd = fs.openSync(filepath, 'r'), + expected = 'xyz\n', + bufferAsync = new Buffer(expected.length), + bufferSync = new Buffer(expected.length), + readCalled = 0; + +fs.read(fd, bufferAsync, 0, expected.length, 0, function(err, bytesRead) { + readCalled++; + + assert.equal(bytesRead, expected.length); + assert.deepEqual(bufferAsync, new Buffer(expected)); +}); + +var r = fs.readSync(fd, bufferSync, 0, expected.length, 0); +assert.deepEqual(bufferSync, new Buffer(expected)); +assert.equal(r, expected.length); + +process.addListener('exit', function() { + assert.equal(readCalled, 1); +}); \ No newline at end of file diff --git a/test/simple/test-fs-read.js b/test/simple/test-fs-read.js new file mode 100644 index 0000000000..7f9bf48d4a --- /dev/null +++ b/test/simple/test-fs-read.js @@ -0,0 +1,23 @@ +require('../common'); +var path = require('path'), + fs = require('fs'), + filepath = path.join(fixturesDir, 'x.txt'), + fd = fs.openSync(filepath, 'r'), + expected = 'xyz\n', + readCalled = 0; + +fs.read(fd, expected.length, 0, 'utf-8', function(err, str, bytesRead) { + readCalled++; + + assert.ok(!err); + assert.equal(str, expected); + assert.equal(bytesRead, expected.length); +}); + +var r = fs.readSync(fd, expected.length, 0, 'utf-8'); +assert.equal(r[0], expected); +assert.equal(r[1], expected.length); + +process.addListener('exit', function() { + assert.equal(readCalled, 1); +}); \ No newline at end of file