From f6825735ec73ca3a9b11afff65c2138bc878375a Mon Sep 17 00:00:00 2001 From: Erik Kemperman Date: Tue, 28 Jun 2016 23:25:17 +0200 Subject: [PATCH] Update: Avoid passing fd to updateMetadata callback (#174) --- lib/dest/write-contents/write-buffer.js | 7 ++-- lib/dest/write-contents/write-dir.js | 7 ++-- lib/file-operations.js | 10 ++--- test/file-operations.js | 50 ++++++++++++------------- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/lib/dest/write-contents/write-buffer.js b/lib/dest/write-contents/write-buffer.js index 54d3fb44..16a3469c 100644 --- a/lib/dest/write-contents/write-buffer.js +++ b/lib/dest/write-contents/write-buffer.js @@ -16,11 +16,12 @@ function writeBuffer(file, written) { } fo.updateMetadata(fd, file, onUpdate); - } - function onUpdate(statErr, fd) { - fo.closeFd(statErr, fd, written); + function onUpdate(statErr) { + fo.closeFd(statErr, fd, written); + } } + } module.exports = writeBuffer; diff --git a/lib/dest/write-contents/write-dir.js b/lib/dest/write-contents/write-dir.js index b7d23019..08f1febb 100644 --- a/lib/dest/write-contents/write-dir.js +++ b/lib/dest/write-contents/write-dir.js @@ -31,11 +31,12 @@ function writeDir(file, written) { } fo.updateMetadata(fd, file, onUpdate); - } - function onUpdate(statErr, fd) { - fo.closeFd(statErr, fd, written); + function onUpdate(statErr) { + fo.closeFd(statErr, fd, written); + } } + } function isInaccessible(err) { diff --git a/lib/file-operations.js b/lib/file-operations.js index cec9efb3..5dbcaf92 100644 --- a/lib/file-operations.js +++ b/lib/file-operations.js @@ -97,7 +97,7 @@ function updateMetadata(fd, file, callback) { function onStat(err, stat) { if (err) { - return callback(err, fd); + return callback(err); } // Check if mode needs to be updated @@ -111,13 +111,13 @@ function updateMetadata(fd, file, callback) { // Nothing to do if (!modeDiff && !timesDiff) { - return callback(null, fd); + return callback(null); } // Check access, `futimes` and `fchmod` only work if we own the file, // or if we are effectively root. if (!isOwner(stat)) { - return callback(null, fd); + return callback(null); } if (modeDiff) { @@ -137,7 +137,7 @@ function updateMetadata(fd, file, callback) { if (timesDiff) { return times(fchmodErr); } - callback(fchmodErr, fd); + callback(fchmodErr); } } @@ -149,7 +149,7 @@ function updateMetadata(fd, file, callback) { file.stat.atime = timesDiff.atime; file.stat.mtime = timesDiff.mtime; } - callback(fchmodErr || futimesErr, fd); + callback(fchmodErr || futimesErr); } } } diff --git a/test/file-operations.js b/test/file-operations.js index 053bbfff..e517e897 100644 --- a/test/file-operations.js +++ b/test/file-operations.js @@ -632,7 +632,7 @@ describe('updateMetadata', function() { done(); }); - it('passes the error and file descriptor if fstat fails', function(done) { + it('passes the error if fstat fails', function(done) { if (isWindows) { console.log('Changing the time of a directory errors in Windows.'); console.log('Changing the mode of a file is not supported by node.js in Windows.'); @@ -643,10 +643,8 @@ describe('updateMetadata', function() { var fd = 9001; - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(err).toExist(); - expect(typeof fd === 'number').toEqual(true); - expect(fd2).toEqual(fd); done(); }); @@ -661,13 +659,13 @@ describe('updateMetadata', function() { var fd = fs.openSync(inputPath, 'w+'); var stats = fs.fstatSync(fd); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { // Not sure why .toEqual doesn't match these Object.keys(file.stat).forEach(function(key) { expect(file.stat[key]).toEqual(stats[key]); }); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -682,11 +680,11 @@ describe('updateMetadata', function() { var fd = fs.openSync(inputPath, 'w+'); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(fchmodSpy.calls.length).toEqual(0); expect(futimesSpy.calls.length).toEqual(0); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -708,11 +706,11 @@ describe('updateMetadata', function() { var fd = fs.openSync(inputPath, 'w+'); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(fchmodSpy.calls.length).toEqual(0); expect(futimesSpy.calls.length).toEqual(0); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -731,7 +729,7 @@ describe('updateMetadata', function() { var fd = fs.openSync(inputPath, 'w+'); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(futimesSpy.calls.length).toEqual(1); var stats = fs.fstatSync(fd); var mtimeMs = Date.parse(file.stat.mtime); @@ -743,7 +741,7 @@ describe('updateMetadata', function() { expect(file.stat.atime).toEqual(new Date(then)); expect(atime).toEqual(Date.parse(stats.atime)); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -763,13 +761,13 @@ describe('updateMetadata', function() { file.stat.atime = new Date(then); var fd = fs.openSync(inputPath, 'w+'); + expect(typeof fd === 'number').toEqual(true); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(err).toExist(); expect(futimesSpy.calls.length).toEqual(1); - expect(typeof fd2 === 'number').toEqual(true); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -786,12 +784,12 @@ describe('updateMetadata', function() { var fd = fs.openSync(inputPath, 'w+'); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(fchmodSpy.calls.length).toEqual(1); var stats = fs.fstatSync(fd); expect(file.stat.mode).toEqual(stats.mode); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -809,12 +807,12 @@ describe('updateMetadata', function() { var fd = fs.openSync(inputPath, 'w+'); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(fchmodSpy.calls.length).toEqual(1); var stats = fs.fstatSync(fd); expect(file.stat.mode).toEqual(stats.mode); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -833,12 +831,11 @@ describe('updateMetadata', function() { cb(new Error('mocked error')); }); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(err).toExist(); expect(fchmodSpy.calls.length).toEqual(1); - expect(typeof fd2 === 'number').toEqual(true); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -861,7 +858,7 @@ describe('updateMetadata', function() { var fd = fs.openSync(inputPath, 'w+'); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(fchmodSpy.calls.length).toEqual(1); expect(futimesSpy.calls.length).toEqual(1); @@ -877,7 +874,7 @@ describe('updateMetadata', function() { expect(atime).toEqual(Date.parse(stats.atime)); expect(file.stat.mode).toEqual(stats.mode); - fs.close(fd2, done); + fs.close(fd, done); }); }); @@ -904,14 +901,13 @@ describe('updateMetadata', function() { var fd = fs.openSync(inputPath, 'w'); - updateMetadata(fd, file, function(err, fd2) { + updateMetadata(fd, file, function(err) { expect(err).toExist(); expect(err).toEqual(expectedErr); expect(fchmodSpy.calls.length).toEqual(1); expect(futimesSpy.calls.length).toEqual(1); - expect(typeof fd2 === 'number').toEqual(true); - fs.close(fd2, done); + fs.close(fd, done); }); }); });