Deprecate string interface for fs.read()
This patch makes buffers the preferred output for fs.read() and fs.readSync(). The old string interface is still supported by converting buffers to strings dynamically. This allows to remove the C++ code for string handling which is also part of this patch.
This commit is contained in:
Felix Geisendörfer
Ryan Dahl
parent
cbd2c3945b
commit
cbbf9e43d1
@ -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)
|
||||
|
||||
|
||||
54
lib/fs.js
54
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) {
|
||||
|
||||
137
src/node_file.cc
137
src/node_file.cc
@ -106,18 +106,9 @@ static int After(eio_req *req) {
|
||||
|
||||
case EIO_READ:
|
||||
{
|
||||
if (req->int3) {
|
||||
// legacy interface
|
||||
Local<Object> obj = Local<Object>::New(*callback);
|
||||
Local<Value> 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<Value> 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<Value> Read(const Arguments& args) {
|
||||
HandleScope scope;
|
||||
@ -605,105 +586,47 @@ static Handle<Value> Read(const Arguments& args) {
|
||||
int fd = args[0]->Int32Value();
|
||||
|
||||
Local<Value> 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<Buffer>(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]);
|
||||
|
||||
buf = NULL; // libeio will allocate and free it.
|
||||
|
||||
cb = args[4];
|
||||
if (!Buffer::HasInstance(args[1])) {
|
||||
return ThrowException(Exception::Error(
|
||||
String::New("Second argument needs to be a buffer")));
|
||||
}
|
||||
|
||||
Buffer * buffer = ObjectWrap::Unwrap<Buffer>(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 (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<Object> obj = cb->ToObject();
|
||||
obj->SetHiddenValue(encoding_symbol, args[3]);
|
||||
}
|
||||
|
||||
if (legacy) assert(buf == NULL);
|
||||
|
||||
|
||||
eio_req *req = eio_read(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(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<Array> 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<Integer> 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<Integer> bytesRead = Integer::New(ret);
|
||||
return scope.Close(bytesRead);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
25
test/simple/test-fs-read-buffer.js
Normal file
25
test/simple/test-fs-read-buffer.js
Normal file
@ -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);
|
||||
});
|
||||
23
test/simple/test-fs-read.js
Normal file
23
test/simple/test-fs-read.js
Normal file
@ -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);
|
||||
});
|
||||
Loading…
x
Reference in New Issue
Block a user