Skip to content

Commit 9b35736

Browse files
erikkempermanphated
authored andcommitted
Update: Avoid passing fd to updateMetadata callback (#174)
1 parent b8fcb3a commit 9b35736

File tree

4 files changed

+36
-38
lines changed

4 files changed

+36
-38
lines changed

lib/dest/write-contents/write-buffer.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ function writeBuffer(file, written) {
1616
}
1717

1818
fo.updateMetadata(fd, file, onUpdate);
19-
}
2019

21-
function onUpdate(statErr, fd) {
22-
fo.closeFd(statErr, fd, written);
20+
function onUpdate(statErr) {
21+
fo.closeFd(statErr, fd, written);
22+
}
2323
}
24+
2425
}
2526

2627
module.exports = writeBuffer;

lib/dest/write-contents/write-dir.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,12 @@ function writeDir(file, written) {
3131
}
3232

3333
fo.updateMetadata(fd, file, onUpdate);
34-
}
3534

36-
function onUpdate(statErr, fd) {
37-
fo.closeFd(statErr, fd, written);
35+
function onUpdate(statErr) {
36+
fo.closeFd(statErr, fd, written);
37+
}
3838
}
39+
3940
}
4041

4142
function isInaccessible(err) {

lib/file-operations.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ function updateMetadata(fd, file, callback) {
9797

9898
function onStat(err, stat) {
9999
if (err) {
100-
return callback(err, fd);
100+
return callback(err);
101101
}
102102

103103
// Check if mode needs to be updated
@@ -111,13 +111,13 @@ function updateMetadata(fd, file, callback) {
111111

112112
// Nothing to do
113113
if (!modeDiff && !timesDiff) {
114-
return callback(null, fd);
114+
return callback(null);
115115
}
116116

117117
// Check access, `futimes` and `fchmod` only work if we own the file,
118118
// or if we are effectively root.
119119
if (!isOwner(stat)) {
120-
return callback(null, fd);
120+
return callback(null);
121121
}
122122

123123
if (modeDiff) {
@@ -137,7 +137,7 @@ function updateMetadata(fd, file, callback) {
137137
if (timesDiff) {
138138
return times(fchmodErr);
139139
}
140-
callback(fchmodErr, fd);
140+
callback(fchmodErr);
141141
}
142142
}
143143

@@ -149,7 +149,7 @@ function updateMetadata(fd, file, callback) {
149149
file.stat.atime = timesDiff.atime;
150150
file.stat.mtime = timesDiff.mtime;
151151
}
152-
callback(fchmodErr || futimesErr, fd);
152+
callback(fchmodErr || futimesErr);
153153
}
154154
}
155155
}

test/file-operations.js

+23-27
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ describe('updateMetadata', function() {
632632
done();
633633
});
634634

635-
it('passes the error and file descriptor if fstat fails', function(done) {
635+
it('passes the error if fstat fails', function(done) {
636636
if (isWindows) {
637637
console.log('Changing the time of a directory errors in Windows.');
638638
console.log('Changing the mode of a file is not supported by node.js in Windows.');
@@ -643,10 +643,8 @@ describe('updateMetadata', function() {
643643

644644
var fd = 9001;
645645

646-
updateMetadata(fd, file, function(err, fd2) {
646+
updateMetadata(fd, file, function(err) {
647647
expect(err).toExist();
648-
expect(typeof fd === 'number').toEqual(true);
649-
expect(fd2).toEqual(fd);
650648

651649
done();
652650
});
@@ -661,13 +659,13 @@ describe('updateMetadata', function() {
661659
var fd = fs.openSync(inputPath, 'w+');
662660
var stats = fs.fstatSync(fd);
663661

664-
updateMetadata(fd, file, function(err, fd2) {
662+
updateMetadata(fd, file, function(err) {
665663
// Not sure why .toEqual doesn't match these
666664
Object.keys(file.stat).forEach(function(key) {
667665
expect(file.stat[key]).toEqual(stats[key]);
668666
});
669667

670-
fs.close(fd2, done);
668+
fs.close(fd, done);
671669
});
672670
});
673671

@@ -682,11 +680,11 @@ describe('updateMetadata', function() {
682680

683681
var fd = fs.openSync(inputPath, 'w+');
684682

685-
updateMetadata(fd, file, function(err, fd2) {
683+
updateMetadata(fd, file, function(err) {
686684
expect(fchmodSpy.calls.length).toEqual(0);
687685
expect(futimesSpy.calls.length).toEqual(0);
688686

689-
fs.close(fd2, done);
687+
fs.close(fd, done);
690688
});
691689
});
692690

@@ -708,11 +706,11 @@ describe('updateMetadata', function() {
708706

709707
var fd = fs.openSync(inputPath, 'w+');
710708

711-
updateMetadata(fd, file, function(err, fd2) {
709+
updateMetadata(fd, file, function(err) {
712710
expect(fchmodSpy.calls.length).toEqual(0);
713711
expect(futimesSpy.calls.length).toEqual(0);
714712

715-
fs.close(fd2, done);
713+
fs.close(fd, done);
716714
});
717715
});
718716

@@ -731,7 +729,7 @@ describe('updateMetadata', function() {
731729

732730
var fd = fs.openSync(inputPath, 'w+');
733731

734-
updateMetadata(fd, file, function(err, fd2) {
732+
updateMetadata(fd, file, function(err) {
735733
expect(futimesSpy.calls.length).toEqual(1);
736734
var stats = fs.fstatSync(fd);
737735
var mtimeMs = Date.parse(file.stat.mtime);
@@ -743,7 +741,7 @@ describe('updateMetadata', function() {
743741
expect(file.stat.atime).toEqual(new Date(then));
744742
expect(atime).toEqual(Date.parse(stats.atime));
745743

746-
fs.close(fd2, done);
744+
fs.close(fd, done);
747745
});
748746
});
749747

@@ -763,13 +761,13 @@ describe('updateMetadata', function() {
763761
file.stat.atime = new Date(then);
764762

765763
var fd = fs.openSync(inputPath, 'w+');
764+
expect(typeof fd === 'number').toEqual(true);
766765

767-
updateMetadata(fd, file, function(err, fd2) {
766+
updateMetadata(fd, file, function(err) {
768767
expect(err).toExist();
769768
expect(futimesSpy.calls.length).toEqual(1);
770-
expect(typeof fd2 === 'number').toEqual(true);
771769

772-
fs.close(fd2, done);
770+
fs.close(fd, done);
773771
});
774772
});
775773

@@ -786,12 +784,12 @@ describe('updateMetadata', function() {
786784

787785
var fd = fs.openSync(inputPath, 'w+');
788786

789-
updateMetadata(fd, file, function(err, fd2) {
787+
updateMetadata(fd, file, function(err) {
790788
expect(fchmodSpy.calls.length).toEqual(1);
791789
var stats = fs.fstatSync(fd);
792790
expect(file.stat.mode).toEqual(stats.mode);
793791

794-
fs.close(fd2, done);
792+
fs.close(fd, done);
795793
});
796794
});
797795

@@ -809,12 +807,12 @@ describe('updateMetadata', function() {
809807

810808
var fd = fs.openSync(inputPath, 'w+');
811809

812-
updateMetadata(fd, file, function(err, fd2) {
810+
updateMetadata(fd, file, function(err) {
813811
expect(fchmodSpy.calls.length).toEqual(1);
814812
var stats = fs.fstatSync(fd);
815813
expect(file.stat.mode).toEqual(stats.mode);
816814

817-
fs.close(fd2, done);
815+
fs.close(fd, done);
818816
});
819817
});
820818

@@ -833,12 +831,11 @@ describe('updateMetadata', function() {
833831
cb(new Error('mocked error'));
834832
});
835833

836-
updateMetadata(fd, file, function(err, fd2) {
834+
updateMetadata(fd, file, function(err) {
837835
expect(err).toExist();
838836
expect(fchmodSpy.calls.length).toEqual(1);
839-
expect(typeof fd2 === 'number').toEqual(true);
840837

841-
fs.close(fd2, done);
838+
fs.close(fd, done);
842839
});
843840
});
844841

@@ -861,7 +858,7 @@ describe('updateMetadata', function() {
861858

862859
var fd = fs.openSync(inputPath, 'w+');
863860

864-
updateMetadata(fd, file, function(err, fd2) {
861+
updateMetadata(fd, file, function(err) {
865862
expect(fchmodSpy.calls.length).toEqual(1);
866863
expect(futimesSpy.calls.length).toEqual(1);
867864

@@ -877,7 +874,7 @@ describe('updateMetadata', function() {
877874
expect(atime).toEqual(Date.parse(stats.atime));
878875
expect(file.stat.mode).toEqual(stats.mode);
879876

880-
fs.close(fd2, done);
877+
fs.close(fd, done);
881878
});
882879
});
883880

@@ -904,14 +901,13 @@ describe('updateMetadata', function() {
904901

905902
var fd = fs.openSync(inputPath, 'w');
906903

907-
updateMetadata(fd, file, function(err, fd2) {
904+
updateMetadata(fd, file, function(err) {
908905
expect(err).toExist();
909906
expect(err).toEqual(expectedErr);
910907
expect(fchmodSpy.calls.length).toEqual(1);
911908
expect(futimesSpy.calls.length).toEqual(1);
912-
expect(typeof fd2 === 'number').toEqual(true);
913909

914-
fs.close(fd2, done);
910+
fs.close(fd, done);
915911
});
916912
});
917913
});

0 commit comments

Comments
 (0)