From 8a9c45a4a974bc39223c441c31e5369134f8e6c2 Mon Sep 17 00:00:00 2001 From: Andreas Lind Date: Sat, 23 Jul 2016 22:43:41 +0200 Subject: [PATCH] fs: Fix default params for fs.write(Sync) Add support for fs.write(fd, buffer, cb) and fs.write(fd, buffer, offset, cb) as documented at https://nodejs.org/api/fs.html#fs_fs_write_fd_data_position_encoding_callback and equivalently for fs.writeSync Update docs and code comments to reflect the implementation. PR-URL: https://github.com/nodejs/node/pull/7856 Reviewed-By: Sam Roberts Reviewed-By: James M Snell Reviewed-By: Brian White Reviewed-By: Yorkie Liu Reviewed-By: Gibson Fahnestock --- doc/api/fs.md | 20 ++--- lib/fs.js | 20 +++-- test/parallel/test-fs-write-buffer.js | 120 +++++++++++++++++++++----- test/parallel/test-fs-write-sync.js | 63 ++++++++++---- 4 files changed, 171 insertions(+), 52 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 1a208102ab3204..f799b869e65c53 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1816,13 +1816,13 @@ _Note: [`fs.watch()`][] is more efficient than `fs.watchFile` and `fs.unwatchFile`. `fs.watch` should be used instead of `fs.watchFile` and `fs.unwatchFile` when possible._ -## fs.write(fd, buffer, offset, length[, position], callback) +## fs.write(fd, buffer[, offset[, length[, position]]], callback) * `fd` {Integer} -* `buffer` {String | Buffer} +* `buffer` {Buffer} * `offset` {Integer} * `length` {Integer} * `position` {Integer} @@ -1847,19 +1847,19 @@ On Linux, positional writes don't work when the file is opened in append mode. The kernel ignores the position argument and always appends the data to the end of the file. -## fs.write(fd, data[, position[, encoding]], callback) +## fs.write(fd, string[, position[, encoding]], callback) * `fd` {Integer} -* `data` {String | Buffer} +* `string` {String} * `position` {Integer} * `encoding` {String} * `callback` {Function} -Write `data` to the file specified by `fd`. If `data` is not a Buffer instance -then the value will be coerced to a string. +Write `string` to the file specified by `fd`. If `string` is not a string, then +the value will be coerced to one. `position` refers to the offset from the beginning of the file where this data should be written. If `typeof position !== 'number'` the data will be written at @@ -1940,24 +1940,24 @@ added: v0.1.29 The synchronous version of [`fs.writeFile()`][]. Returns `undefined`. -## fs.writeSync(fd, buffer, offset, length[, position]) +## fs.writeSync(fd, buffer[, offset[, length[, position]]]) * `fd` {Integer} -* `buffer` {String | Buffer} +* `buffer` {Buffer} * `offset` {Integer} * `length` {Integer} * `position` {Integer} -## fs.writeSync(fd, data[, position[, encoding]]) +## fs.writeSync(fd, string[, position[, encoding]]) * `fd` {Integer} -* `data` {String | Buffer} +* `string` {String} * `position` {Integer} * `encoding` {String} diff --git a/lib/fs.js b/lib/fs.js index a241e6c456e4a7..27614e5cf1febc 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -662,7 +662,7 @@ fs.readSync = function(fd, buffer, offset, length, position) { }; // usage: -// fs.write(fd, buffer, offset, length[, position], callback); +// fs.write(fd, buffer[, offset[, length[, position]]], callback); // OR // fs.write(fd, string[, position[, encoding]], callback); fs.write = function(fd, buffer, offset, length, position, callback) { @@ -675,12 +675,16 @@ fs.write = function(fd, buffer, offset, length, position, callback) { req.oncomplete = wrapper; if (buffer instanceof Buffer) { - // if no position is passed then assume null - if (typeof position === 'function') { - callback = position; + callback = maybeCallback(callback || position || length || offset); + if (typeof offset !== 'number') { + offset = 0; + } + if (typeof length !== 'number') { + length = buffer.length - offset; + } + if (typeof position !== 'number') { position = null; } - callback = maybeCallback(callback); return binding.writeBuffer(fd, buffer, offset, length, position, req); } @@ -700,13 +704,17 @@ fs.write = function(fd, buffer, offset, length, position, callback) { }; // usage: -// fs.writeSync(fd, buffer, offset, length[, position]); +// fs.writeSync(fd, buffer[, offset[, length[, position]]]); // OR // fs.writeSync(fd, string[, position[, encoding]]); fs.writeSync = function(fd, buffer, offset, length, position) { if (buffer instanceof Buffer) { if (position === undefined) position = null; + if (typeof offset !== 'number') + offset = 0; + if (typeof length !== 'number') + length = buffer.length - offset; return binding.writeBuffer(fd, buffer, offset, length, position); } if (typeof buffer !== 'string') diff --git a/test/parallel/test-fs-write-buffer.js b/test/parallel/test-fs-write-buffer.js index b6404a036a78c2..ed77d697b33bd2 100644 --- a/test/parallel/test-fs-write-buffer.js +++ b/test/parallel/test-fs-write-buffer.js @@ -2,29 +2,107 @@ const common = require('../common'); const assert = require('assert'); const path = require('path'); -const Buffer = require('buffer').Buffer; const fs = require('fs'); -const filename = path.join(common.tmpDir, 'write.txt'); const expected = Buffer.from('hello'); common.refreshTmpDir(); -fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) { - if (err) throw err; - - fs.write(fd, - expected, - 0, - expected.length, - null, - common.mustCall(function(err, written) { - if (err) throw err; - - assert.equal(expected.length, written); - fs.closeSync(fd); - - var found = fs.readFileSync(filename, 'utf8'); - assert.deepStrictEqual(expected.toString(), found); - fs.unlinkSync(filename); - })); -})); +// fs.write with all parameters provided: +{ + const filename = path.join(common.tmpDir, 'write1.txt'); + fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) { + assert.ifError(err); + + const cb = common.mustCall(function(err, written) { + assert.ifError(err); + + assert.strictEqual(expected.length, written); + fs.closeSync(fd); + + var found = fs.readFileSync(filename, 'utf8'); + assert.deepStrictEqual(expected.toString(), found); + }); + + fs.write(fd, expected, 0, expected.length, null, cb); + })); +} + +// fs.write with a buffer, without the length parameter: +{ + const filename = path.join(common.tmpDir, 'write2.txt'); + fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) { + assert.ifError(err); + + const cb = common.mustCall(function(err, written) { + assert.ifError(err); + + assert.strictEqual(2, written); + fs.closeSync(fd); + + const found = fs.readFileSync(filename, 'utf8'); + assert.deepStrictEqual('lo', found); + }); + + fs.write(fd, Buffer.from('hello'), 3, cb); + })); +} + +// fs.write with a buffer, without the offset and length parameters: +{ + const filename = path.join(common.tmpDir, 'write3.txt'); + fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) { + assert.ifError(err); + + const cb = common.mustCall(function(err, written) { + assert.ifError(err); + + assert.strictEqual(expected.length, written); + fs.closeSync(fd); + + const found = fs.readFileSync(filename, 'utf8'); + assert.deepStrictEqual(expected.toString(), found); + }); + + fs.write(fd, expected, cb); + })); +} + +// fs.write with the offset passed as undefined followed by the callback: +{ + const filename = path.join(common.tmpDir, 'write4.txt'); + fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) { + assert.ifError(err); + + const cb = common.mustCall(function(err, written) { + assert.ifError(err); + + assert.strictEqual(expected.length, written); + fs.closeSync(fd); + + const found = fs.readFileSync(filename, 'utf8'); + assert.deepStrictEqual(expected.toString(), found); + }); + + fs.write(fd, expected, undefined, cb); + })); +} + +// fs.write with offset and length passed as undefined followed by the callback: +{ + const filename = path.join(common.tmpDir, 'write5.txt'); + fs.open(filename, 'w', 0o644, common.mustCall(function(err, fd) { + assert.ifError(err); + + const cb = common.mustCall(function(err, written) { + assert.ifError(err); + + assert.strictEqual(expected.length, written); + fs.closeSync(fd); + + const found = fs.readFileSync(filename, 'utf8'); + assert.deepStrictEqual(expected.toString(), found); + }); + + fs.write(fd, expected, undefined, undefined, cb); + })); +} diff --git a/test/parallel/test-fs-write-sync.js b/test/parallel/test-fs-write-sync.js index 12f34f81d8e06b..f663d4a5bfe5f5 100644 --- a/test/parallel/test-fs-write-sync.js +++ b/test/parallel/test-fs-write-sync.js @@ -1,23 +1,56 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var path = require('path'); -var fs = require('fs'); -var fn = path.join(common.tmpDir, 'write.txt'); +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const filename = path.join(common.tmpDir, 'write.txt'); common.refreshTmpDir(); -var foo = 'foo'; -var fd = fs.openSync(fn, 'w'); +// fs.writeSync with all parameters provided: +{ + const fd = fs.openSync(filename, 'w'); -var written = fs.writeSync(fd, ''); -assert.strictEqual(0, written); + let written = fs.writeSync(fd, ''); + assert.strictEqual(0, written); -fs.writeSync(fd, foo); + fs.writeSync(fd, 'foo'); -var bar = 'bár'; -written = fs.writeSync(fd, Buffer.from(bar), 0, Buffer.byteLength(bar)); -assert.ok(written > 3); -fs.closeSync(fd); + written = fs.writeSync(fd, Buffer.from('bár'), 0, Buffer.byteLength('bár')); + assert.ok(written > 3); + fs.closeSync(fd); -assert.equal(fs.readFileSync(fn), 'foobár'); + assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár'); +} + +// fs.writeSync with a buffer, without the length parameter: +{ + const fd = fs.openSync(filename, 'w'); + + let written = fs.writeSync(fd, ''); + assert.strictEqual(0, written); + + fs.writeSync(fd, 'foo'); + + written = fs.writeSync(fd, Buffer.from('bár'), 0); + assert.ok(written > 3); + fs.closeSync(fd); + + assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár'); +} + +// fs.writeSync with a buffer, without the offset and length parameters: +{ + const fd = fs.openSync(filename, 'w'); + + let written = fs.writeSync(fd, ''); + assert.strictEqual(0, written); + + fs.writeSync(fd, 'foo'); + + written = fs.writeSync(fd, Buffer.from('bár')); + assert.ok(written > 3); + fs.closeSync(fd); + + assert.strictEqual(fs.readFileSync(filename, 'utf-8'), 'foobár'); +}