Skip to content

Commit

Permalink
fs: remove fs.read's string interface
Browse files Browse the repository at this point in the history
It is a maintenance burden that was removed from the docs in 2010
(c93e0aa) and runtime-deprecated in v6.0
(1124de2).

PR-URL: #9683
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
seishun committed Jan 10, 2017
1 parent 2685464 commit 3c2a936
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 218 deletions.
82 changes: 2 additions & 80 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -555,40 +555,7 @@ fs.openSync = function(path, flags, mode) {
return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);
};

var readWarned = false;
fs.read = function(fd, buffer, offset, length, position, callback) {
if (!isUint8Array(buffer)) {
// legacy string interface (fd, length, position, encoding, callback)
if (!readWarned) {
readWarned = true;
process.emitWarning(
'fs.read\'s legacy String interface is deprecated. Use the Buffer ' +
'API as mentioned in the documentation instead.',
'DeprecationWarning');
}

const cb = arguments[4];
const encoding = arguments[3];

assertEncoding(encoding);

position = arguments[2];
length = arguments[1];
buffer = Buffer.allocUnsafe(length);
offset = 0;

callback = function(err, bytesRead) {
if (!cb) return;
if (err) return cb(err);

if (bytesRead > 0) {
tryToStringWithEnd(buffer, encoding, bytesRead, cb);
} else {
(cb)(err, '', bytesRead);
}
};
}

if (length === 0) {
return process.nextTick(function() {
callback && callback(null, 0, buffer);
Expand All @@ -606,57 +573,12 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
binding.read(fd, buffer, offset, length, position, req);
};

function tryToStringWithEnd(buf, encoding, end, callback) {
var e;
try {
buf = buf.toString(encoding, 0, end);
} catch (err) {
e = err;
}
callback(e, buf, end);
}

var readSyncWarned = false;
fs.readSync = function(fd, buffer, offset, length, position) {
var legacy = false;
var encoding;

if (!isUint8Array(buffer)) {
// legacy string interface (fd, length, position, encoding, callback)
if (!readSyncWarned) {
readSyncWarned = true;
process.emitWarning(
'fs.readSync\'s legacy String interface is deprecated. Use the ' +
'Buffer API as mentioned in the documentation instead.',
'DeprecationWarning');
}
legacy = true;
encoding = arguments[3];

assertEncoding(encoding);

position = arguments[2];
length = arguments[1];
buffer = Buffer.allocUnsafe(length);

offset = 0;
}

if (length === 0) {
if (legacy) {
return ['', 0];
} else {
return 0;
}
}

var r = binding.read(fd, buffer, offset, length, position);
if (!legacy) {
return r;
return 0;
}

var str = (r > 0) ? buffer.toString(encoding, 0, r) : '';
return [str, r];
return binding.read(fd, buffer, offset, length, position);
};

// usage:
Expand Down
64 changes: 0 additions & 64 deletions test/parallel/test-fs-read-buffer-tostring-fail.js

This file was deleted.

19 changes: 0 additions & 19 deletions test/parallel/test-fs-read-buffer-zero-length.js

This file was deleted.

35 changes: 0 additions & 35 deletions test/parallel/test-fs-read-buffer.js

This file was deleted.

21 changes: 21 additions & 0 deletions test/parallel/test-fs-read-type.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const path = require('path');
const fs = require('fs');
const filepath = path.join(common.fixturesDir, 'x.txt');
const fd = fs.openSync(filepath, 'r');
const expected = 'xyz\n';

// Error must be thrown with string
assert.throws(() => {
fs.read(fd,
expected.length,
0,
'utf-8',
() => {});
}, /Second argument needs to be a buffer/);

assert.throws(() => {
fs.readSync(fd, expected.length, 0, 'utf-8');
}, /Second argument needs to be a buffer/);
15 changes: 8 additions & 7 deletions test/parallel/test-fs-read-zero-length.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@
const common = require('../common');
const assert = require('assert');
const path = require('path');
const Buffer = require('buffer').Buffer;
const fs = require('fs');
const filepath = path.join(common.fixturesDir, 'x.txt');
const fd = fs.openSync(filepath, 'r');
const expected = '';
const bufferAsync = Buffer.alloc(0);
const bufferSync = Buffer.alloc(0);

fs.read(fd, 0, 0, 'utf-8', common.mustCall(function(err, str, bytesRead) {
assert.ok(!err);
assert.equal(str, expected);
fs.read(fd, bufferAsync, 0, 0, 0, common.mustCall(function(err, bytesRead) {
assert.equal(bytesRead, 0);
assert.deepStrictEqual(bufferAsync, Buffer.alloc(0));
}));

const r = fs.readSync(fd, 0, 0, 'utf-8');
assert.equal(r[0], expected);
assert.equal(r[1], 0);
const r = fs.readSync(fd, bufferSync, 0, 0, 0);
assert.deepStrictEqual(bufferSync, Buffer.alloc(0));
assert.equal(r, 0);
39 changes: 26 additions & 13 deletions test/parallel/test-fs-read.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,34 @@
const common = require('../common');
const assert = require('assert');
const path = require('path');
const Buffer = require('buffer').Buffer;
const fs = require('fs');
const filepath = path.join(common.fixturesDir, 'x.txt');
const fd = fs.openSync(filepath, 'r');
const expected = 'xyz\n';

fs.read(fd,
expected.length,
0,
'utf-8',
common.mustCall((err, str, bytesRead) => {
assert.ifError(err);
assert.strictEqual(str, expected);
assert.strictEqual(bytesRead, expected.length);
}));
const expected = Buffer.from('xyz\n');

var r = fs.readSync(fd, expected.length, 0, 'utf-8');
assert.strictEqual(r[0], expected);
assert.strictEqual(r[1], expected.length);
function test(bufferAsync, bufferSync, expected) {
fs.read(fd,
bufferAsync,
0,
expected.length,
0,
common.mustCall((err, bytesRead) => {
assert.ifError(err);
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(bufferAsync, Buffer.from(expected));
}));

const r = fs.readSync(fd, bufferSync, 0, expected.length, 0);
assert.deepStrictEqual(bufferSync, Buffer.from(expected));
assert.strictEqual(r, expected.length);
}

test(Buffer.allocUnsafe(expected.length),
Buffer.allocUnsafe(expected.length),
expected);

test(new Uint8Array(expected.length),
new Uint8Array(expected.length),
Uint8Array.from(expected));

0 comments on commit 3c2a936

Please sign in to comment.