Skip to content

Commit f7049a2

Browse files
committed
fs: refactor stats array to be more generic
- Pass kFsStatsFieldsLength between JS and C++ instead of using the magic number 14 - Pass the global stats array to the completion callback of asynchronous FSReqWrap similar to how the stats arrays are passed to the FSReqPromise resolvers - Abstract the stats converter and take an offset to compute the old stats in fs.watchFile - Use templates in node::FillStatsArray and FSReqPromise in preparation for BigInt intergration - Put the global stat array filler in node_internals.h because it is shared by node_file.cc and node_stat_watcher.cc PR-URL: #19714 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 30fe55e commit f7049a2

File tree

9 files changed

+168
-163
lines changed

9 files changed

+168
-163
lines changed

lib/fs.js

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ const {
4343
} = errors.codes;
4444
const { Readable, Writable } = require('stream');
4545
const EventEmitter = require('events');
46-
const { FSReqWrap, statValues } = binding;
46+
const { FSReqWrap, statValues, kFsStatsFieldsLength } = binding;
4747
const { FSEvent } = process.binding('fs_event_wrap');
4848
const internalFS = require('internal/fs');
4949
const { getPathFromURL } = require('internal/url');
@@ -56,7 +56,8 @@ const {
5656
nullCheck,
5757
preprocessSymlinkDestination,
5858
Stats,
59-
statsFromValues,
59+
getStatsFromBinding,
60+
getStatsFromGlobalBinding,
6061
stringToFlags,
6162
stringToSymlinkType,
6263
toUnixTimestamp,
@@ -145,9 +146,9 @@ function makeStatsCallback(cb) {
145146
throw new ERR_INVALID_CALLBACK();
146147
}
147148

148-
return function(err) {
149+
return function(err, stats) {
149150
if (err) return cb(err);
150-
cb(err, statsFromValues());
151+
cb(err, getStatsFromBinding(stats));
151152
};
152153
}
153154

@@ -891,7 +892,7 @@ fs.fstatSync = function(fd) {
891892
const ctx = { fd };
892893
binding.fstat(fd, undefined, ctx);
893894
handleErrorFromBinding(ctx);
894-
return statsFromValues();
895+
return getStatsFromGlobalBinding();
895896
};
896897

897898
fs.lstatSync = function(path) {
@@ -900,7 +901,7 @@ fs.lstatSync = function(path) {
900901
const ctx = { path };
901902
binding.lstat(pathModule.toNamespacedPath(path), undefined, ctx);
902903
handleErrorFromBinding(ctx);
903-
return statsFromValues();
904+
return getStatsFromGlobalBinding();
904905
};
905906

906907
fs.statSync = function(path) {
@@ -909,7 +910,7 @@ fs.statSync = function(path) {
909910
const ctx = { path };
910911
binding.stat(pathModule.toNamespacedPath(path), undefined, ctx);
911912
handleErrorFromBinding(ctx);
912-
return statsFromValues();
913+
return getStatsFromGlobalBinding();
913914
};
914915

915916
fs.readlink = function(path, options, callback) {
@@ -1420,15 +1421,6 @@ function emitStop(self) {
14201421
self.emit('stop');
14211422
}
14221423

1423-
function statsFromPrevValues() {
1424-
return new Stats(statValues[14], statValues[15], statValues[16],
1425-
statValues[17], statValues[18], statValues[19],
1426-
statValues[20] < 0 ? undefined : statValues[20],
1427-
statValues[21], statValues[22],
1428-
statValues[23] < 0 ? undefined : statValues[23],
1429-
statValues[24], statValues[25], statValues[26],
1430-
statValues[27]);
1431-
}
14321424
function StatWatcher() {
14331425
EventEmitter.call(this);
14341426

@@ -1439,13 +1431,14 @@ function StatWatcher() {
14391431
// the sake of backwards compatibility
14401432
var oldStatus = -1;
14411433

1442-
this._handle.onchange = function(newStatus) {
1434+
this._handle.onchange = function(newStatus, stats) {
14431435
if (oldStatus === -1 &&
14441436
newStatus === -1 &&
14451437
statValues[2/* new nlink */] === statValues[16/* old nlink */]) return;
14461438

14471439
oldStatus = newStatus;
1448-
self.emit('change', statsFromValues(), statsFromPrevValues());
1440+
self.emit('change', getStatsFromBinding(stats),
1441+
getStatsFromBinding(stats, kFsStatsFieldsLength));
14491442
};
14501443

14511444
this._handle.onstop = function() {

lib/fs/promises.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ const { isUint8Array } = require('internal/util/types');
2323
const {
2424
copyObject,
2525
getOptions,
26+
getStatsFromBinding,
2627
isUint32,
2728
modeNum,
2829
nullCheck,
2930
preprocessSymlinkDestination,
30-
statsFromValues,
3131
stringToFlags,
3232
stringToSymlinkType,
3333
toUnixTimestamp,
@@ -332,21 +332,24 @@ async function symlink(target, path, type_) {
332332

333333
async function fstat(handle) {
334334
validateFileHandle(handle);
335-
return statsFromValues(await binding.fstat(handle.fd, kUsePromises));
335+
const result = await binding.fstat(handle.fd, kUsePromises);
336+
return getStatsFromBinding(result);
336337
}
337338

338339
async function lstat(path) {
339340
path = getPathFromURL(path);
340341
validatePath(path);
341-
return statsFromValues(
342-
await binding.lstat(pathModule.toNamespacedPath(path), kUsePromises));
342+
const result = await binding.lstat(pathModule.toNamespacedPath(path),
343+
kUsePromises);
344+
return getStatsFromBinding(result);
343345
}
344346

345347
async function stat(path) {
346348
path = getPathFromURL(path);
347349
validatePath(path);
348-
return statsFromValues(
349-
await binding.stat(pathModule.toNamespacedPath(path), kUsePromises));
350+
const result = await binding.stat(pathModule.toNamespacedPath(path),
351+
kUsePromises);
352+
return getStatsFromBinding(result);
350353
}
351354

352355
async function link(existingPath, newPath) {

lib/internal/fs.js

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ function preprocessSymlinkDestination(path, type, linkPath) {
134134
}
135135
}
136136

137+
function dateFromNumeric(num) {
138+
return new Date(num + 0.5);
139+
}
140+
137141
// Constructor for file stats.
138142
function Stats(
139143
dev,
@@ -165,10 +169,10 @@ function Stats(
165169
this.mtimeMs = mtim_msec;
166170
this.ctimeMs = ctim_msec;
167171
this.birthtimeMs = birthtim_msec;
168-
this.atime = new Date(atim_msec + 0.5);
169-
this.mtime = new Date(mtim_msec + 0.5);
170-
this.ctime = new Date(ctim_msec + 0.5);
171-
this.birthtime = new Date(birthtim_msec + 0.5);
172+
this.atime = dateFromNumeric(atim_msec);
173+
this.mtime = dateFromNumeric(mtim_msec);
174+
this.ctime = dateFromNumeric(ctim_msec);
175+
this.birthtime = dateFromNumeric(birthtim_msec);
172176
}
173177

174178
Stats.prototype._checkModeProperty = function(property) {
@@ -203,11 +207,18 @@ Stats.prototype.isSocket = function() {
203207
return this._checkModeProperty(S_IFSOCK);
204208
};
205209

206-
function statsFromValues(stats = statValues) {
207-
return new Stats(stats[0], stats[1], stats[2], stats[3], stats[4], stats[5],
208-
stats[6] < 0 ? undefined : stats[6], stats[7], stats[8],
209-
stats[9] < 0 ? undefined : stats[9], stats[10], stats[11],
210-
stats[12], stats[13]);
210+
function getStatsFromBinding(stats, offset = 0) {
211+
return new Stats(stats[0 + offset], stats[1 + offset], stats[2 + offset],
212+
stats[3 + offset], stats[4 + offset], stats[5 + offset],
213+
stats[6 + offset] < 0 ? undefined : stats[6 + offset],
214+
stats[7 + offset], stats[8 + offset],
215+
stats[9 + offset] < 0 ? undefined : stats[9 + offset],
216+
stats[10 + offset], stats[11 + offset],
217+
stats[12 + offset], stats[13 + offset]);
218+
}
219+
220+
function getStatsFromGlobalBinding(offset = 0) {
221+
return getStatsFromBinding(statValues, offset);
211222
}
212223

213224
function stringToFlags(flags) {
@@ -424,7 +435,8 @@ module.exports = {
424435
nullCheck,
425436
preprocessSymlinkDestination,
426437
realpathCacheKey: Symbol('realpathCacheKey'),
427-
statsFromValues,
438+
getStatsFromBinding,
439+
getStatsFromGlobalBinding,
428440
stringToFlags,
429441
stringToSymlinkType,
430442
Stats,

src/env.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ Environment::Environment(IsolateData* isolate_data,
104104
#endif
105105
handle_cleanup_waiting_(0),
106106
http_parser_buffer_(nullptr),
107-
fs_stats_field_array_(isolate_, kFsStatsFieldsLength),
107+
fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2),
108108
context_(context->GetIsolate(), context) {
109109
// We'll be creating new objects so make sure we've entered the context.
110110
v8::HandleScope handle_scope(isolate());

src/env.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,10 @@ class Environment {
644644

645645
inline AliasedBuffer<double, v8::Float64Array>* fs_stats_field_array();
646646

647+
// stat fields contains twice the number of entries because `fs.StatWatcher`
648+
// needs room to store data for *two* `fs.Stats` instances.
649+
static const int kFsStatsFieldsLength = 14;
650+
647651
inline std::vector<std::unique_ptr<fs::FileHandleReadWrap>>&
648652
file_handle_read_wrap_freelist();
649653

@@ -822,9 +826,6 @@ class Environment {
822826
bool http_parser_buffer_in_use_ = false;
823827
std::unique_ptr<http2::http2_state> http2_state_;
824828

825-
// stat fields contains twice the number of entries because `fs.StatWatcher`
826-
// needs room to store data for *two* `fs.Stats` instances.
827-
static const int kFsStatsFieldsLength = 2 * 14;
828829
AliasedBuffer<double, v8::Float64Array> fs_stats_field_array_;
829830

830831
std::vector<std::unique_ptr<fs::FileHandleReadWrap>>

0 commit comments

Comments
 (0)