From f7a3f8eff5c2b1069436c0e4657e006789c9c96e Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Tue, 7 Jan 2020 16:21:42 +1100 Subject: [PATCH 01/11] fs: add statfs to fs Add support for statfs system call to the fs module using uv_fs_statfs() function from libuv. Refs: https://github.com/nodejs/node/issues/10745 --- doc/api/fs.md | 128 +++++++++++++++++++++++++++++- lib/fs.js | 38 +++++++++ lib/internal/fs/promises.js | 9 +++ lib/internal/fs/utils.js | 20 +++++ src/env-inl.h | 8 ++ src/env.cc | 5 ++ src/env.h | 24 ++++++ src/node_file-inl.h | 47 ++++++++++- src/node_file.cc | 46 +++++++++++ src/node_file.h | 12 +++ test/parallel/test-fs-promises.js | 19 +++++ test/parallel/test-fs-statfs.js | 41 ++++++++++ tools/doc/type-parser.js | 1 + 13 files changed, 396 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-fs-statfs.js diff --git a/doc/api/fs.md b/doc/api/fs.md index 4421d80b3341ff..b51005e10353bb 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1001,6 +1001,90 @@ The times in the stat object have the following semantics: Prior to Node.js 0.12, the `ctime` held the `birthtime` on Windows systems. As of 0.12, `ctime` is not "creation time", and on Unix systems, it never was. +## Class: `fs.StatFs` + +Provides information about a mounted filesystem. + +Objects returned from [`fs.statfs()`][] and its synchronous counterpart are of +this type. If `bigint` in the `options` passed to those methods is true, the +numeric values will be `bigint` instead of `number`. + +```console +StatFs { + type: 1397114950, + bsize: 4096, + blocks: 121938943, + bfree: 61058895, + bavail: 61058895, + files: 999, + ffree: 1000000, + spare: [ 0, 0, 0, 0 ] +} +``` + +`bigint` version: + +```console +StatFs { + type: 1397114950n, + bsize: 4096n, + blocks: 121938943n, + bfree: 61058895n, + bavail: 61058895n, + files: 999n, + ffree: 1000000n, + spare: [ 0n, 0n, 0n, 0n ] +} +``` + +### `statfs.type` + +* {number|bigint} + +Type of filesystem. + +### `statfs.bsize` + +* {number|bigint} + +Optimal transfer block size. + +### `statfs.blocks` + +* {number|bigint} + +Total data blocks in filesystem. + +### `statfs.bfree` + +* {number|bigint} + +Free blocks in filesystem. + +### `statfs.bavail` + +* {number|bigint} + +Free blocks available to unprivileged user. + +### `statfs.files` + +* {number|bigint} + +Total file nodes in filesystem. + +### `statfs.ffree` + +* {number|bigint} + +Free file nodes in filesystem. + +### `statfs.spare` + +* {Array} + +Padding bytes reserved for future use. + ## Class: `fs.WriteStream` +* `path` {string|Buffer|URL} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.StatFs`][] object should be `bigint`. **Default:** `false`. +* `callback` {Function} + * `err` {Error} + * `stats` {fs.StatFs} + +Asynchronous statfs(2). The callback gets two arguments `(err, stats)` where +`stats` is an [`fs.StatFs`][] object. + +Returns information about the mounted filesystem which contains `path`. + +In case of an error, the `err.code` will be one of [Common System Errors][]. + +## `fs.statfsSync(path[, options])` +* `path` {string|Buffer|URL} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.StatFs`][] object should be `bigint`. **Default:** `false`. +* Returns: {fs.StatFs} + +Synchronous statfs(2). + ## `fs.symlink(target, path[, type], callback)` @@ -5586,6 +5710,7 @@ the file contents. [`fs.Dirent`]: #fs_class_fs_dirent [`fs.FSWatcher`]: #fs_class_fs_fswatcher [`fs.Stats`]: #fs_class_fs_stats +[`fs.StatFs`]: #fs_class_fs_statfs [`fs.access()`]: #fs_fs_access_path_mode_callback [`fs.chmod()`]: #fs_fs_chmod_path_mode_callback [`fs.chown()`]: #fs_fs_chown_path_uid_gid_callback @@ -5609,6 +5734,7 @@ the file contents. [`fs.realpath()`]: #fs_fs_realpath_path_options_callback [`fs.rmdir()`]: #fs_fs_rmdir_path_options_callback [`fs.stat()`]: #fs_fs_stat_path_options_callback +[`fs.statfs()`]: #fs_fs_statfs_path_options_callback [`fs.symlink()`]: #fs_fs_symlink_target_path_type_callback [`fs.utimes()`]: #fs_fs_utimes_path_atime_mtime_callback [`fs.watch()`]: #fs_fs_watch_filename_options_listener diff --git a/lib/fs.js b/lib/fs.js index a85b8ab29ebd1a..9a19b9facb8cc3 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -76,6 +76,7 @@ const { Dirent, getDirents, getOptions, + getStatFsFromBinding, getValidatedPath, getValidMode, handleErrorFromBinding, @@ -176,6 +177,19 @@ function makeStatsCallback(cb) { }; } +// Special case of `makeCallback()` specific to async `statfs()`. Transforms +// the array passed to the callback to a javascript object +function makeStatfsCallback(cb) { + if (typeof cb !== 'function') { + throw new ERR_INVALID_CALLBACK(cb); + } + + return (err, statsArray) => { + if (err) return cb(err); + cb(err, getStatFsFromBinding(statsArray)); + }; +} + const isFd = isUint32; function isFileType(stats, fileType) { @@ -936,6 +950,19 @@ function stat(path, options = { bigint: false }, callback) { binding.stat(pathModule.toNamespacedPath(path), options.bigint, req); } +function statfs(path, options = { bigint: false }, callback) { + if (typeof options === 'function') { + callback = options; + options = {}; + } + + callback = makeStatfsCallback(callback); + path = getValidatedPath(path); + const req = new FSReqCallback(options.bigint); + req.oncomplete = callback; + binding.statfs(pathModule.toNamespacedPath(path), options.bigint, req); +} + function fstatSync(fd, options = { bigint: false }) { validateInt32(fd, 'fd', 0); const ctx = { fd }; @@ -962,6 +989,15 @@ function statSync(path, options = { bigint: false }) { return getStatsFromBinding(stats); } +function statfsSync(path, options = { bigint: false }) { + path = getValidatedPath(path); + const ctx = { path }; + const stats = binding.statfs(pathModule.toNamespacedPath(path), + options.bigint, undefined, ctx); + handleErrorFromBinding(ctx); + return getStatFsFromBinding(stats); +} + function readlink(path, options, callback) { callback = makeCallback(typeof options === 'function' ? options : callback); options = getOptions(options, {}); @@ -1920,7 +1956,9 @@ module.exports = fs = { rmdir, rmdirSync, stat, + statfs, statSync, + statfsSync, symlink, symlinkSync, truncate, diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 6517a5ef4be056..91c7c9d0a9eef7 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -33,6 +33,7 @@ const { getDirents, getOptions, getStatsFromBinding, + getStatFsFromBinding, getValidatedPath, getValidMode, nullCheck, @@ -405,6 +406,13 @@ async function stat(path, options = { bigint: false }) { return getStatsFromBinding(result); } +async function statfs(path, options = { bigint: false }) { + path = getValidatedPath(path); + const result = await binding.statfs(pathModule.toNamespacedPath(path), + options.bigint, kUsePromises); + return getStatFsFromBinding(result); +} + async function link(existingPath, newPath) { existingPath = getValidatedPath(existingPath, 'existingPath'); newPath = getValidatedPath(newPath, 'newPath'); @@ -536,6 +544,7 @@ module.exports = { symlink, lstat, stat, + statfs, link, unlink, chmod, diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 763a940f6e98a7..d172a559fb5c3c 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -458,6 +458,25 @@ function getStatsFromBinding(stats, offset = 0) { ); } +function StatFs(type, bsize, blocks, bfree, bavail, files, ffree, + spare1, spare2, spare3, spare4) { + this.type = type; + this.bsize = bsize; + this.blocks = blocks; + this.bfree = bfree; + this.bavail = bavail; + this.files = files; + this.ffree = ffree; + this.spare = [ spare1, spare2, spare3, spare4 ]; +} + +function getStatFsFromBinding(stats) { + return new StatFs( + stats[0], stats[1], stats[2], stats[3], stats[4], stats[5], + stats[6], stats[7], stats[8], stats[9], stats[10] + ); +} + function stringToFlags(flags) { if (typeof flags === 'number') { return flags; @@ -669,6 +688,7 @@ module.exports = { getDirent, getDirents, getOptions, + getStatFsFromBinding, getValidatedPath, getValidMode, handleErrorFromBinding, diff --git a/src/env-inl.h b/src/env-inl.h index 3c7b83795d723a..f54dcb5346c150 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -615,6 +615,14 @@ inline AliasedBigUint64Array* Environment::fs_stats_field_bigint_array() { return &fs_stats_field_bigint_array_; } +inline AliasedFloat64Array* Environment::fs_statfs_field_array() { + return &fs_statfs_field_array_; +} + +inline AliasedBigUint64Array* Environment::fs_statfs_field_bigint_array() { + return &fs_statfs_field_bigint_array_; +} + inline std::vector>& Environment::file_handle_read_wrap_freelist() { return file_handle_read_wrap_freelist_; diff --git a/src/env.cc b/src/env.cc index 36b41c5fac56c4..d8d6dbed9e87ed 100644 --- a/src/env.cc +++ b/src/env.cc @@ -309,6 +309,8 @@ Environment::Environment(IsolateData* isolate_data, thread_id_(thread_id == kNoThreadId ? AllocateThreadId() : thread_id), fs_stats_field_array_(isolate_, kFsStatsBufferLength), fs_stats_field_bigint_array_(isolate_, kFsStatsBufferLength), + fs_statfs_field_array_(isolate_, kFsStatfsBufferLength), + fs_statfs_field_bigint_array_(isolate_, kFsStatfsBufferLength), context_(context->GetIsolate(), context) { // We'll be creating new objects so make sure we've entered the context. HandleScope handle_scope(isolate()); @@ -1068,6 +1070,9 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("fs_stats_field_array", fs_stats_field_array_); tracker->TrackField("fs_stats_field_bigint_array", fs_stats_field_bigint_array_); + tracker->TrackField("fs_statfs_field_array", fs_statfs_field_array_); + tracker->TrackField("fs_statfs_field_bigint_array", + fs_statfs_field_bigint_array_); tracker->TrackField("cleanup_hooks", cleanup_hooks_); tracker->TrackField("async_hooks", async_hooks_); tracker->TrackField("immediate_info", immediate_info_); diff --git a/src/env.h b/src/env.h index b1ec84bd75d1ba..aadc24987abe2c 100644 --- a/src/env.h +++ b/src/env.h @@ -135,6 +135,24 @@ enum class FsStatsOffset { constexpr size_t kFsStatsBufferLength = static_cast(FsStatsOffset::kFsStatsFieldsNumber) * 2; +enum class FsStatfsOffset { + kType = 0, + kBSize, + kBlocks, + kBFree, + kBAvail, + kFiles, + kFFree, + kSpare1, + kSpare2, + kSpare3, + kSpare4, + kFsStatFsFieldsNumber +}; + +constexpr size_t kFsStatfsBufferLength = + static_cast(FsStatfsOffset::kFsStatFsFieldsNumber); + // PER_ISOLATE_* macros: We have a lot of per-isolate properties // and adding and maintaining their getters and setters by hand would be // difficult so let's make the preprocessor generate them for us. @@ -1025,6 +1043,9 @@ class Environment : public MemoryRetainer { inline AliasedFloat64Array* fs_stats_field_array(); inline AliasedBigUint64Array* fs_stats_field_bigint_array(); + inline AliasedFloat64Array* fs_statfs_field_array(); + inline AliasedBigUint64Array* fs_statfs_field_bigint_array(); + inline std::vector>& file_handle_read_wrap_freelist(); @@ -1386,6 +1407,9 @@ class Environment : public MemoryRetainer { AliasedFloat64Array fs_stats_field_array_; AliasedBigUint64Array fs_stats_field_bigint_array_; + AliasedFloat64Array fs_statfs_field_array_; + AliasedBigUint64Array fs_statfs_field_bigint_array_; + std::vector> file_handle_read_wrap_freelist_; diff --git a/src/node_file-inl.h b/src/node_file-inl.h index e9ed18a75fa48b..a31a6e5d09a7cb 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -129,6 +129,41 @@ v8::Local FillGlobalStatsArray(Environment* env, } } +template +void FillStatfsArray(AliasedBufferBase* fields, + const uv_statfs_t* s) { +#define SET_FIELD(field, stat) \ + fields->SetValue(static_cast(FsStatfsOffset::field), \ + static_cast(stat)) + + SET_FIELD(kType, s->f_type); + SET_FIELD(kBSize, s->f_bsize); + SET_FIELD(kBlocks, s->f_blocks); + SET_FIELD(kBFree, s->f_bfree); + SET_FIELD(kBAvail, s->f_bavail); + SET_FIELD(kFiles, s->f_files); + SET_FIELD(kFFree, s->f_ffree); + SET_FIELD(kSpare1, s->f_spare[0]); + SET_FIELD(kSpare2, s->f_spare[1]); + SET_FIELD(kSpare3, s->f_spare[2]); + SET_FIELD(kSpare4, s->f_spare[3]); +#undef SET_FIELD +} + +v8::Local FillGlobalStatfsArray(Environment* env, + const bool use_bigint, + const uv_statfs_t* s) { + if (use_bigint) { + auto* const arr = env->fs_statfs_field_bigint_array(); + FillStatfsArray(arr, s); + return arr->GetJSArray(); + } else { + auto* const arr = env->fs_statfs_field_array(); + FillStatfsArray(arr, s); + return arr->GetJSArray(); + } +} + template FSReqPromise* FSReqPromise::New(Environment* env, bool use_bigint) { @@ -160,7 +195,10 @@ FSReqPromise::FSReqPromise( : FSReqBase(env, obj, AsyncWrap::PROVIDER_FSREQPROMISE, use_bigint), stats_field_array_( env->isolate(), - static_cast(FsStatsOffset::kFsStatsFieldsNumber)) {} + static_cast(FsStatsOffset::kFsStatsFieldsNumber)), + statfs_field_array_( + env->isolate(), + static_cast(FsStatfsOffset::kFsStatFsFieldsNumber)) {} template void FSReqPromise::Reject(v8::Local reject) { @@ -192,6 +230,12 @@ void FSReqPromise::ResolveStat(const uv_stat_t* stat) { Resolve(stats_field_array_.GetJSArray()); } +template +void FSReqPromise::ResolveStatfs(const uv_statfs_t* stat) { + FillStatfsArray(&statfs_field_array_, stat); + Resolve(statfs_field_array_.GetJSArray()); +} + template void FSReqPromise::SetReturnValue( const v8::FunctionCallbackInfo& args) { @@ -206,6 +250,7 @@ template void FSReqPromise::MemoryInfo(MemoryTracker* tracker) const { FSReqBase::MemoryInfo(tracker); tracker->TrackField("stats_field_array", stats_field_array_); + tracker->TrackField("statfs_field_array", statfs_field_array_); } FSReqBase* GetReqWrap(Environment* env, v8::Local value, diff --git a/src/node_file.cc b/src/node_file.cc index 2f66080e5caa14..2b0a46c99b6af6 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -502,6 +502,10 @@ void FSReqCallback::ResolveStat(const uv_stat_t* stat) { Resolve(FillGlobalStatsArray(env(), use_bigint(), stat)); } +void FSReqCallback::ResolveStatfs(const uv_statfs_t* stat) { + Resolve(FillGlobalStatfsArray(env(), use_bigint(), stat)); +} + void FSReqCallback::Resolve(Local value) { Local argv[2] { Null(env()->isolate()), @@ -578,6 +582,15 @@ void AfterStat(uv_fs_t* req) { } } +void AfterStatfs(uv_fs_t* req) { + FSReqBase* req_wrap = FSReqBase::from_req(req); + FSReqAfterScope after(req_wrap, req); + + if (after.Proceed()) { + req_wrap->ResolveStatfs(reinterpret_cast(req->ptr)); + } +} + void AfterInteger(uv_fs_t* req) { FSReqBase* req_wrap = FSReqBase::from_req(req); FSReqAfterScope after(req_wrap, req); @@ -1020,6 +1033,38 @@ static void FStat(const FunctionCallbackInfo& args) { } } +static void Statfs(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + const int argc = args.Length(); + CHECK_GE(argc, 2); + + BufferValue path(env->isolate(), args[0]); + CHECK_NOT_NULL(*path); + + bool use_bigint = args[1]->IsTrue(); + FSReqBase* req_wrap_async = GetReqWrap(env, args[2], use_bigint); + if (req_wrap_async != nullptr) { // statfs(path, use_bigint, req) + AsyncCall(env, req_wrap_async, args, "statfs", UTF8, AfterStatfs, + uv_fs_statfs, *path); + } else { + CHECK_EQ(argc, 4); + FSReqWrapSync req_wrap_sync; + FS_SYNC_TRACE_BEGIN(statfs); + int err = SyncCall(env, args[3], &req_wrap_sync, "statfs", uv_fs_statfs, + *path); + FS_SYNC_TRACE_END(statfs); + + if (err != 0) { + return; // error info is in ctx + } + + Local arr = FillGlobalStatfsArray(env, use_bigint, + static_cast(req_wrap_sync.req.ptr)); + args.GetReturnValue().Set(arr); + } +} + static void Symlink(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); @@ -2254,6 +2299,7 @@ void Initialize(Local target, env->SetMethod(target, "stat", Stat); env->SetMethod(target, "lstat", LStat); env->SetMethod(target, "fstat", FStat); + env->SetMethod(target, "statfs", Statfs); env->SetMethod(target, "link", Link); env->SetMethod(target, "symlink", Symlink); env->SetMethod(target, "readlink", ReadLink); diff --git a/src/node_file.h b/src/node_file.h index 1ebfe06c1fbdbf..a8f28ade7e9a94 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -59,6 +59,7 @@ class FSReqBase : public ReqWrap { virtual void Reject(v8::Local reject) = 0; virtual void Resolve(v8::Local value) = 0; virtual void ResolveStat(const uv_stat_t* stat) = 0; + virtual void ResolveStatfs(const uv_statfs_t* stat) = 0; virtual void SetReturnValue( const v8::FunctionCallbackInfo& args) = 0; @@ -104,6 +105,7 @@ class FSReqCallback final : public FSReqBase { void Reject(v8::Local reject) override; void Resolve(v8::Local value) override; void ResolveStat(const uv_stat_t* stat) override; + void ResolveStatfs(const uv_statfs_t* stat) override; void SetReturnValue(const v8::FunctionCallbackInfo& args) override; SET_MEMORY_INFO_NAME(FSReqCallback) @@ -123,6 +125,14 @@ inline v8::Local FillGlobalStatsArray(Environment* env, const uv_stat_t* s, const bool second = false); +template +void FillStatfsArray(AliasedBufferBase* fields, + const uv_statfs_t* s); + +inline v8::Local FillGlobalStatfsArray(Environment* env, + const bool use_bigint, + const uv_statfs_t* s); + template class FSReqPromise final : public FSReqBase { public: @@ -132,6 +142,7 @@ class FSReqPromise final : public FSReqBase { inline void Reject(v8::Local reject) override; inline void Resolve(v8::Local value) override; inline void ResolveStat(const uv_stat_t* stat) override; + inline void ResolveStatfs(const uv_statfs_t* stat) override; inline void SetReturnValue( const v8::FunctionCallbackInfo& args) override; inline void MemoryInfo(MemoryTracker* tracker) const override; @@ -151,6 +162,7 @@ class FSReqPromise final : public FSReqBase { bool finished_ = false; AliasedBufferT stats_field_array_; + AliasedBufferT statfs_field_array_; }; class FSReqAfterScope final { diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index f23df685eac072..d93162f1dcfe2e 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -26,6 +26,7 @@ const { rename, rmdir, stat, + statfs, symlink, truncate, unlink, @@ -82,6 +83,18 @@ function verifyStatObject(stat) { assert.strictEqual(typeof stat.mode, 'number'); } +function verifyStatfsObject(stat) { + assert.strictEqual(typeof stat, 'object'); + assert.strictEqual(typeof stat.type, 'number'); + assert.strictEqual(typeof stat.bsize, 'number'); + assert.strictEqual(typeof stat.blocks, 'number'); + assert.strictEqual(typeof stat.bfree, 'number'); + assert.strictEqual(typeof stat.bavail, 'number'); + assert.strictEqual(typeof stat.files, 'number'); + assert.strictEqual(typeof stat.ffree, 'number'); + assert.ok(stat.spare instanceof Array); +} + async function getHandle(dest) { await copyFile(fixtures.path('baz.js'), dest); await access(dest); @@ -126,6 +139,12 @@ async function getHandle(dest) { await handle.close(); } + // file system stats + { + const statFs = await statfs(dest); + verifyStatfsObject(statFs); + } + // Test fs.read promises when length to read is zero bytes { const dest = path.resolve(tmpDir, 'test1.js'); diff --git a/test/parallel/test-fs-statfs.js b/test/parallel/test-fs-statfs.js new file mode 100644 index 00000000000000..32447d1adeeeb7 --- /dev/null +++ b/test/parallel/test-fs-statfs.js @@ -0,0 +1,41 @@ +'use strict'; +const common = require('../common'); + +const assert = require('assert'); +const fs = require('fs'); + +fs.statfs(__filename, common.mustCall(function(err, stats) { + assert.ifError(err); + [ + 'type', 'bsize', 'blocks', 'bfree', 'bavail', 'files', 'ffree' + ].forEach((k) => { + assert.ok(stats.hasOwnProperty(k)); + assert.strictEqual(typeof stats[k], 'number', `${k} should be a number`); + }); + assert.ok(stats.hasOwnProperty('spare')); + assert.ok(stats.spare instanceof Array); + // Confirm that we are not running in the context of the internal binding + // layer. + // Ref: https://github.com/nodejs/node/commit/463d6bac8b349acc462d345a6e298a76f7d06fb1 + assert.strictEqual(this, undefined); +})); + +[false, 1, {}, [], null, undefined].forEach((input) => { + assert.throws( + () => fs.statfs(input, common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + } + ); + assert.throws( + () => fs.statfsSync(input), + { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + } + ); +}); + +// Should not throw an error +fs.statfs(__filename, undefined, common.mustCall(() => {})); diff --git a/tools/doc/type-parser.js b/tools/doc/type-parser.js index 02b59d37ffd278..19fad88a70700a 100644 --- a/tools/doc/type-parser.js +++ b/tools/doc/type-parser.js @@ -79,6 +79,7 @@ const customTypesMap = { 'fs.FSWatcher': 'fs.html#fs_class_fs_fswatcher', 'fs.ReadStream': 'fs.html#fs_class_fs_readstream', 'fs.Stats': 'fs.html#fs_class_fs_stats', + 'fs.StatFs': 'fs.html#fs_class_fs_statfs', 'fs.WriteStream': 'fs.html#fs_class_fs_writestream', 'http.Agent': 'http.html#http_class_http_agent', From 7e69ae3362bebbcc6b23e2dc648164899eba20a5 Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Tue, 14 Jan 2020 17:10:10 +1100 Subject: [PATCH 02/11] Fix doc --- doc/api/fs.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index b51005e10353bb..2e3448e50eaa83 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -5236,10 +5236,10 @@ changes: The `Promise` is resolved with the [`fs.Stats`][] object for the given `path`. -<<<<<<< HEAD - ### `fsPromises.statfs(path[, options])` - + * `path` {string|Buffer|URL} * `options` {Object} * `bigint` {boolean} Whether the numeric values in the returned From 3a10e4539ca727616c882bee69baa9b1e7a9a561 Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Tue, 14 Jan 2020 21:29:57 +1100 Subject: [PATCH 03/11] Fix typos in doc --- doc/api/fs.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 2e3448e50eaa83..d0c03a019e4048 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1006,7 +1006,7 @@ of 0.12, `ctime` is not "creation time", and on Unix systems, it never was. Provides information about a mounted filesystem. Objects returned from [`fs.statfs()`][] and its synchronous counterpart are of -this type. If `bigint` in the `options` passed to those methods is true, the +this type. If `bigint` in the `options` passed to those methods is `true`, the numeric values will be `bigint` instead of `number`. ```console @@ -5248,7 +5248,7 @@ added: REPLACEME The `Promise` is resolved with the [`fs.StatFs`][] object for the given `path`. -### `fsPromises.symlink(target, path\[, type\])` +### `fsPromises.symlink(target, path[, type])` From 56665b84e08a50c7ba3780c87d39ff50a28ff595 Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Tue, 14 Jan 2020 22:17:28 +1100 Subject: [PATCH 04/11] Move oncomplete callback inline --- lib/fs.js | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 9a19b9facb8cc3..f91c1dce6f3c24 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -177,19 +177,6 @@ function makeStatsCallback(cb) { }; } -// Special case of `makeCallback()` specific to async `statfs()`. Transforms -// the array passed to the callback to a javascript object -function makeStatfsCallback(cb) { - if (typeof cb !== 'function') { - throw new ERR_INVALID_CALLBACK(cb); - } - - return (err, statsArray) => { - if (err) return cb(err); - cb(err, getStatFsFromBinding(statsArray)); - }; -} - const isFd = isUint32; function isFileType(stats, fileType) { @@ -956,10 +943,17 @@ function statfs(path, options = { bigint: false }, callback) { options = {}; } - callback = makeStatfsCallback(callback); + if (typeof callback !== 'function') { + throw new ERR_INVALID_CALLBACK(callback); + } + + const oncompleteCallback = (err, statsArray) => { + if (err) return callback(err); + callback(err, getStatFsFromBinding(statsArray)); + }; path = getValidatedPath(path); const req = new FSReqCallback(options.bigint); - req.oncomplete = callback; + req.oncomplete = oncompleteCallback; binding.statfs(pathModule.toNamespacedPath(path), options.bigint, req); } From 5313e93be012a4b67eb0ace7ac672d254c3d85db Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Wed, 15 Jan 2020 02:53:07 +1100 Subject: [PATCH 05/11] Remove spare field(s) --- lib/internal/fs/utils.js | 9 ++++----- src/env.h | 4 ---- src/node_file-inl.h | 8 ++------ src/node_file.cc | 2 +- test/parallel/test-fs-promises.js | 1 - test/parallel/test-fs-statfs.js | 2 -- 6 files changed, 7 insertions(+), 19 deletions(-) diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index d172a559fb5c3c..acdf0842b711a6 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -458,8 +458,8 @@ function getStatsFromBinding(stats, offset = 0) { ); } -function StatFs(type, bsize, blocks, bfree, bavail, files, ffree, - spare1, spare2, spare3, spare4) { +function StatFs(type, bsize, blocks, bfree, + bavail, files, ffree) { this.type = type; this.bsize = bsize; this.blocks = blocks; @@ -467,13 +467,12 @@ function StatFs(type, bsize, blocks, bfree, bavail, files, ffree, this.bavail = bavail; this.files = files; this.ffree = ffree; - this.spare = [ spare1, spare2, spare3, spare4 ]; } function getStatFsFromBinding(stats) { return new StatFs( - stats[0], stats[1], stats[2], stats[3], stats[4], stats[5], - stats[6], stats[7], stats[8], stats[9], stats[10] + stats[0], stats[1], stats[2], stats[3], + stats[4], stats[5], stats[6] ); } diff --git a/src/env.h b/src/env.h index aadc24987abe2c..eba1819cb17bd5 100644 --- a/src/env.h +++ b/src/env.h @@ -143,10 +143,6 @@ enum class FsStatfsOffset { kBAvail, kFiles, kFFree, - kSpare1, - kSpare2, - kSpare3, - kSpare4, kFsStatFsFieldsNumber }; diff --git a/src/node_file-inl.h b/src/node_file-inl.h index a31a6e5d09a7cb..c8ac9d8d4c38cd 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -143,16 +143,12 @@ void FillStatfsArray(AliasedBufferBase* fields, SET_FIELD(kBAvail, s->f_bavail); SET_FIELD(kFiles, s->f_files); SET_FIELD(kFFree, s->f_ffree); - SET_FIELD(kSpare1, s->f_spare[0]); - SET_FIELD(kSpare2, s->f_spare[1]); - SET_FIELD(kSpare3, s->f_spare[2]); - SET_FIELD(kSpare4, s->f_spare[3]); #undef SET_FIELD } v8::Local FillGlobalStatfsArray(Environment* env, - const bool use_bigint, - const uv_statfs_t* s) { + const bool use_bigint, + const uv_statfs_t* s) { if (use_bigint) { auto* const arr = env->fs_statfs_field_bigint_array(); FillStatfsArray(arr, s); diff --git a/src/node_file.cc b/src/node_file.cc index 2b0a46c99b6af6..09e1500c5c20c3 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -587,7 +587,7 @@ void AfterStatfs(uv_fs_t* req) { FSReqAfterScope after(req_wrap, req); if (after.Proceed()) { - req_wrap->ResolveStatfs(reinterpret_cast(req->ptr)); + req_wrap->ResolveStatfs(static_cast(req->ptr)); } } diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index d93162f1dcfe2e..670ecb817bdaf3 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -92,7 +92,6 @@ function verifyStatfsObject(stat) { assert.strictEqual(typeof stat.bavail, 'number'); assert.strictEqual(typeof stat.files, 'number'); assert.strictEqual(typeof stat.ffree, 'number'); - assert.ok(stat.spare instanceof Array); } async function getHandle(dest) { diff --git a/test/parallel/test-fs-statfs.js b/test/parallel/test-fs-statfs.js index 32447d1adeeeb7..01a93c0cf0585e 100644 --- a/test/parallel/test-fs-statfs.js +++ b/test/parallel/test-fs-statfs.js @@ -12,8 +12,6 @@ fs.statfs(__filename, common.mustCall(function(err, stats) { assert.ok(stats.hasOwnProperty(k)); assert.strictEqual(typeof stats[k], 'number', `${k} should be a number`); }); - assert.ok(stats.hasOwnProperty('spare')); - assert.ok(stats.spare instanceof Array); // Confirm that we are not running in the context of the internal binding // layer. // Ref: https://github.com/nodejs/node/commit/463d6bac8b349acc462d345a6e298a76f7d06fb1 From 490d268c8ae7849acafb1f56c22357094897a7a8 Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Wed, 15 Jan 2020 02:53:52 +1100 Subject: [PATCH 06/11] Update and fix doc --- doc/api/fs.md | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index d0c03a019e4048..e34cb27063c6dd 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1017,8 +1017,7 @@ StatFs { bfree: 61058895, bavail: 61058895, files: 999, - ffree: 1000000, - spare: [ 0, 0, 0, 0 ] + ffree: 1000000 } ``` @@ -1032,8 +1031,7 @@ StatFs { bfree: 61058895n, bavail: 61058895n, files: 999n, - ffree: 1000000n, - spare: [ 0n, 0n, 0n, 0n ] + ffree: 1000000n } ``` @@ -1079,12 +1077,6 @@ Total file nodes in filesystem. Free file nodes in filesystem. -### `statfs.spare` - -* {Array} - -Padding bytes reserved for future use. - ## Class: `fs.WriteStream` + * `path` {string|Buffer|URL} * `options` {Object} * `bigint` {boolean} Whether the numeric values in the returned @@ -3569,6 +3562,10 @@ Returns information about the mounted filesystem which contains `path`. In case of an error, the `err.code` will be one of [Common System Errors][]. ## `fs.statfsSync(path[, options])` + + * `path` {string|Buffer|URL} * `options` {Object} * `bigint` {boolean} Whether the numeric values in the returned @@ -5240,6 +5237,7 @@ The `Promise` is resolved with the [`fs.Stats`][] object for the given `path`. + * `path` {string|Buffer|URL} * `options` {Object} * `bigint` {boolean} Whether the numeric values in the returned From dc6a4a0a461657a8dc1f31d3909446120ffa4c69 Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Wed, 15 Jan 2020 12:01:45 +1100 Subject: [PATCH 07/11] Use maybeCallback --- lib/fs.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index f91c1dce6f3c24..1c4eee3d223f8a 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -943,9 +943,7 @@ function statfs(path, options = { bigint: false }, callback) { options = {}; } - if (typeof callback !== 'function') { - throw new ERR_INVALID_CALLBACK(callback); - } + callback = maybeCallback(callback); const oncompleteCallback = (err, statsArray) => { if (err) return callback(err); From c27332b4a79e598f5a519edbaa50125dc8853236 Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Wed, 15 Jan 2020 12:03:07 +1100 Subject: [PATCH 08/11] Add tests for statfsSync and bigint varations --- test/parallel/test-fs-promises.js | 24 ++++++++++++++------- test/parallel/test-fs-statfs.js | 35 +++++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index 670ecb817bdaf3..34af70fccaa07b 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -83,15 +83,17 @@ function verifyStatObject(stat) { assert.strictEqual(typeof stat.mode, 'number'); } -function verifyStatfsObject(stat) { +function verifyStatfsObject(stat, isBigint = false) { + const valueType = isBigint ? 'bigint' : 'number'; + assert.strictEqual(typeof stat, 'object'); - assert.strictEqual(typeof stat.type, 'number'); - assert.strictEqual(typeof stat.bsize, 'number'); - assert.strictEqual(typeof stat.blocks, 'number'); - assert.strictEqual(typeof stat.bfree, 'number'); - assert.strictEqual(typeof stat.bavail, 'number'); - assert.strictEqual(typeof stat.files, 'number'); - assert.strictEqual(typeof stat.ffree, 'number'); + assert.strictEqual(typeof stat.type, valueType); + assert.strictEqual(typeof stat.bsize, valueType); + assert.strictEqual(typeof stat.blocks, valueType); + assert.strictEqual(typeof stat.bfree, valueType); + assert.strictEqual(typeof stat.bavail, valueType); + assert.strictEqual(typeof stat.files, valueType); + assert.strictEqual(typeof stat.ffree, valueType); } async function getHandle(dest) { @@ -144,6 +146,12 @@ async function getHandle(dest) { verifyStatfsObject(statFs); } + // file system stats bigint + { + const statFs = await statfs(dest, { bigint: true }); + verifyStatfsObject(statFs, true); + } + // Test fs.read promises when length to read is zero bytes { const dest = path.resolve(tmpDir, 'test1.js'); diff --git a/test/parallel/test-fs-statfs.js b/test/parallel/test-fs-statfs.js index 01a93c0cf0585e..aa368d150292d4 100644 --- a/test/parallel/test-fs-statfs.js +++ b/test/parallel/test-fs-statfs.js @@ -4,20 +4,47 @@ const common = require('../common'); const assert = require('assert'); const fs = require('fs'); -fs.statfs(__filename, common.mustCall(function(err, stats) { - assert.ifError(err); +function verifyStatFsObject(statfs, isBigint = false) { + const valueType = isBigint ? 'bigint' : 'number'; + [ 'type', 'bsize', 'blocks', 'bfree', 'bavail', 'files', 'ffree' ].forEach((k) => { - assert.ok(stats.hasOwnProperty(k)); - assert.strictEqual(typeof stats[k], 'number', `${k} should be a number`); + assert.ok(statfs.hasOwnProperty(k)); + assert.strictEqual(typeof statfs[k], valueType, `${k} should be a ${valueType}`); }); +} + +fs.statfs(__filename, common.mustCall(function(err, stats) { + assert.ifError(err); + verifyStatFsObject(stats); // Confirm that we are not running in the context of the internal binding // layer. // Ref: https://github.com/nodejs/node/commit/463d6bac8b349acc462d345a6e298a76f7d06fb1 assert.strictEqual(this, undefined); })); +fs.statfs(__filename, { bigint: true }, function(err, stats) { + assert.ifError(err); + verifyStatFsObject(stats, true); + // Confirm that we are not running in the context of the internal binding + // layer. + // Ref: https://github.com/nodejs/node/commit/463d6bac8b349acc462d345a6e298a76f7d06fb1 + assert.strictEqual(this, undefined); +}); + +// Synchronous +{ + const statFsObj = fs.statfsSync(__filename); + verifyStatFsObject(statFsObj); +} + +// Synchronous Bigint +{ + const statFsBigIntObj = fs.statfsSync(__filename, { bigint: true }); + verifyStatFsObject(statFsBigIntObj, true); +} + [false, 1, {}, [], null, undefined].forEach((input) => { assert.throws( () => fs.statfs(input, common.mustNotCall()), From eed4efb5a4f185631b11d9c266f15cb3f6500c32 Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Wed, 15 Jan 2020 12:27:17 +1100 Subject: [PATCH 09/11] Fix linting errors --- test/parallel/test-fs-promises.js | 6 +++--- test/parallel/test-fs-statfs.js | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index 34af70fccaa07b..99d7266d66cc61 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -85,7 +85,7 @@ function verifyStatObject(stat) { function verifyStatfsObject(stat, isBigint = false) { const valueType = isBigint ? 'bigint' : 'number'; - + assert.strictEqual(typeof stat, 'object'); assert.strictEqual(typeof stat.type, valueType); assert.strictEqual(typeof stat.bsize, valueType); @@ -140,13 +140,13 @@ async function getHandle(dest) { await handle.close(); } - // file system stats + // File system stats { const statFs = await statfs(dest); verifyStatfsObject(statFs); } - // file system stats bigint + // File system stats bigint { const statFs = await statfs(dest, { bigint: true }); verifyStatfsObject(statFs, true); diff --git a/test/parallel/test-fs-statfs.js b/test/parallel/test-fs-statfs.js index aa368d150292d4..2b6afb0c3fc677 100644 --- a/test/parallel/test-fs-statfs.js +++ b/test/parallel/test-fs-statfs.js @@ -6,12 +6,13 @@ const fs = require('fs'); function verifyStatFsObject(statfs, isBigint = false) { const valueType = isBigint ? 'bigint' : 'number'; - + [ 'type', 'bsize', 'blocks', 'bfree', 'bavail', 'files', 'ffree' ].forEach((k) => { assert.ok(statfs.hasOwnProperty(k)); - assert.strictEqual(typeof statfs[k], valueType, `${k} should be a ${valueType}`); + assert.strictEqual(typeof statfs[k], valueType, + `${k} should be a ${valueType}`); }); } From 8fcea1ea3e336cd5e38f31e04b76a4270b01bede Mon Sep 17 00:00:00 2001 From: Sk Sajidul Kadir Date: Thu, 16 Jan 2020 20:43:57 +1100 Subject: [PATCH 10/11] Put StatFs properties in sorted order in docs --- doc/api/fs.md | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index e34cb27063c6dd..3a724b94882119 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1011,13 +1011,13 @@ numeric values will be `bigint` instead of `number`. ```console StatFs { - type: 1397114950, - bsize: 4096, - blocks: 121938943, - bfree: 61058895, bavail: 61058895, + bfree: 61058895, + blocks: 121938943, + bsize: 4096, + ffree: 1000000, files: 999, - ffree: 1000000 + type: 1397114950 } ``` @@ -1025,27 +1025,27 @@ StatFs { ```console StatFs { - type: 1397114950n, - bsize: 4096n, - blocks: 121938943n, - bfree: 61058895n, bavail: 61058895n, + bfree: 61058895n, + blocks: 121938943n, + bsize: 4096n, + ffree: 1000000n, files: 999n, - ffree: 1000000n + type: 1397114950n } ``` -### `statfs.type` +### `statfs.bavail` * {number|bigint} -Type of filesystem. +Free blocks available to unprivileged user. -### `statfs.bsize` +### `statfs.bfree` * {number|bigint} -Optimal transfer block size. +Free blocks in filesystem. ### `statfs.blocks` @@ -1053,17 +1053,17 @@ Optimal transfer block size. Total data blocks in filesystem. -### `statfs.bfree` +### `statfs.bsize` * {number|bigint} -Free blocks in filesystem. +Optimal transfer block size. -### `statfs.bavail` +### `statfs.ffree` * {number|bigint} -Free blocks available to unprivileged user. +Free file nodes in filesystem. ### `statfs.files` @@ -1071,11 +1071,11 @@ Free blocks available to unprivileged user. Total file nodes in filesystem. -### `statfs.ffree` +### `statfs.type` * {number|bigint} -Free file nodes in filesystem. +Type of filesystem. ## Class: `fs.WriteStream`