Skip to content

Commit 07e4fb2

Browse files
authored
[FS] Make fstatfs actually work (#23381)
Prior to this change, `fstatfs` calls `statfs64` with the `char*` path pointer argument set to zero. Generally byte zero of the memory seems to be null, so when `statfs64` converts this `char*` to a JS string it gets the empty string. Then it calls `FS.statfs` with the empty string as an argument. `FS.statfs` calls `lookupPath` with an empty string argument which returns a `null` node, so `fstatfs` returns the default values for when there is no statfs handler in the files system. This fixes the problem and adds testing for statfs/fstatfs on nodefs and rawfs. We have to add separate `FS.statfsNode` and `FS.statfsStream` handlers because in some cases `stream.node` is null and in some cases `stream.path` is useless (like for stdout). Also, in noderawfs we need to give paths for the standard streams because there is unfortunately no `fs.fstatfsSync` so we can only access information from the host via a path. This means the noderawfs test won't be portable to windows.
1 parent ebfeaf7 commit 07e4fb2

File tree

5 files changed

+56
-25
lines changed

5 files changed

+56
-25
lines changed

src/library_fs.js

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -693,9 +693,18 @@ FS.staticInit();
693693
return parent.node_ops.mknod(parent, name, mode, dev);
694694
},
695695
statfs(path) {
696-
696+
return FS.statfsNode(FS.lookupPath(path, {follow: true}).node);
697+
},
698+
statfsStream(stream) {
699+
// We keep a separate statfsStream function because noderawfs overrides
700+
// it. In noderawfs, stream.node is sometimes null. Instead, we need to
701+
// look at stream.path.
702+
return FS.statfsNode(stream.node);
703+
},
704+
statfsNode(node) {
697705
// NOTE: None of the defaults here are true. We're just returning safe and
698-
// sane values.
706+
// sane values. Currently nodefs and rawfs replace these defaults,
707+
// other file systems leave them alone.
699708
var rtn = {
700709
bsize: 4096,
701710
frsize: 4096,
@@ -709,9 +718,8 @@ FS.staticInit();
709718
namelen: 255,
710719
};
711720

712-
var parent = FS.lookupPath(path, {follow: true}).node;
713-
if (parent?.node_ops.statfs) {
714-
Object.assign(rtn, parent.node_ops.statfs(parent.mount.opts.root));
721+
if (node.node_ops.statfs) {
722+
Object.assign(rtn, node.node_ops.statfs(node.mount.opts.root));
715723
}
716724
return rtn;
717725
},

src/library_noderawfs.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ addToLibrary({
4848
},
4949
createStandardStreams() {
5050
// FIXME: tty is set to true to appease isatty(), the underlying ioctl syscalls still needs to be implemented, see issue #22264.
51-
FS.createStream({ nfd: 0, position: 0, path: '', flags: 0, tty: true, seekable: false }, 0);
51+
FS.createStream({ nfd: 0, position: 0, path: '/dev/stdin', flags: 0, tty: true, seekable: false }, 0);
52+
var paths = [,'/dev/stdout', '/dev/stderr'];
5253
for (var i = 1; i < 3; i++) {
53-
FS.createStream({ nfd: i, position: 0, path: '', flags: {{{ cDefs.O_TRUNC | cDefs.O_CREAT | cDefs.O_WRONLY }}}, tty: true, seekable: false }, i);
54+
FS.createStream({ nfd: i, position: 0, path: paths[i], flags: {{{ cDefs.O_TRUNC | cDefs.O_CREAT | cDefs.O_WRONLY }}}, tty: true, seekable: false }, i);
5455
}
5556
},
5657
// generic function for all node creation
@@ -79,6 +80,12 @@ addToLibrary({
7980
}
8081
return stat;
8182
},
83+
statfsStream(stream) {
84+
return fs.statfsSync(stream.path);
85+
},
86+
statfsNode(node) {
87+
return fs.statfsSync(node.path);
88+
},
8289
chmod(path, mode, dontFollow) {
8390
mode &= {{{ cDefs.S_IALLUGO }}};
8491
if (NODEFS.isWindows) {

src/library_syscall.js

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@ var SyscallsLibrary = {
6363
{{{ makeSetValue('buf', C_STRUCTS.stat.st_ino, 'stat.ino', 'i64') }}};
6464
return 0;
6565
},
66+
writeStatFs(buf, stats) {
67+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_bsize, 'stats.bsize', 'i32') }}};
68+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_frsize, 'stats.bsize', 'i32') }}};
69+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_blocks, 'stats.blocks', 'i32') }}};
70+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_bfree, 'stats.bfree', 'i32') }}};
71+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_bavail, 'stats.bavail', 'i32') }}};
72+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_files, 'stats.files', 'i32') }}};
73+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_ffree, 'stats.ffree', 'i32') }}};
74+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_fsid, 'stats.fsid', 'i32') }}};
75+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_flags, 'stats.flags', 'i32') }}}; // ST_NOSUID
76+
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_namelen, 'stats.namelen', 'i32') }}};
77+
},
6678
doMsync(addr, stream, len, flags, offset) {
6779
if (!FS.isFile(stream.node.mode)) {
6880
throw new FS.ErrnoError({{{ cDefs.ENODEV }}});
@@ -801,23 +813,17 @@ var SyscallsLibrary = {
801813
#if ASSERTIONS
802814
assert(size === {{{ C_STRUCTS.statfs.__size__ }}});
803815
#endif
804-
var stats = FS.statfs(SYSCALLS.getStr(path));
805-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_bsize, 'stats.bsize', 'i32') }}};
806-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_frsize, 'stats.bsize', 'i32') }}};
807-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_blocks, 'stats.blocks', 'i32') }}};
808-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_bfree, 'stats.bfree', 'i32') }}};
809-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_bavail, 'stats.bavail', 'i32') }}};
810-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_files, 'stats.files', 'i32') }}};
811-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_ffree, 'stats.ffree', 'i32') }}};
812-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_fsid, 'stats.fsid', 'i32') }}};
813-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_flags, 'stats.flags', 'i32') }}}; // ST_NOSUID
814-
{{{ makeSetValue('buf', C_STRUCTS.statfs.f_namelen, 'stats.namelen', 'i32') }}};
816+
SYSCALLS.writeStatFs(buf, FS.statfs(SYSCALLS.getStr(path)));
815817
return 0;
816818
},
817819
__syscall_fstatfs64__deps: ['__syscall_statfs64'],
818820
__syscall_fstatfs64: (fd, size, buf) => {
821+
#if ASSERTIONS
822+
assert(size === {{{ C_STRUCTS.statfs.__size__ }}});
823+
#endif
819824
var stream = SYSCALLS.getStreamFromFD(fd);
820-
return ___syscall_statfs64(0, size, buf);
825+
SYSCALLS.writeStatFs(buf, FS.statfsStream(stream));
826+
return 0;
821827
},
822828
__syscall_fadvise64__nothrow: true,
823829
__syscall_fadvise64__proxy: 'none',

test/test_other.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
from common import create_file, parameterized, NON_ZERO, node_pthreads, TEST_ROOT, test_file
3737
from common import compiler_for, EMBUILDER, requires_v8, requires_node, requires_wasm64, requires_node_canary
3838
from common import requires_wasm_eh, crossplatform, with_all_eh_sjlj, with_all_sjlj
39-
from common import also_with_standalone_wasm, also_with_wasm2js, also_with_noderawfs, also_with_wasmfs
39+
from common import also_with_standalone_wasm, also_with_wasm2js, also_with_noderawfs, also_with_wasmfs, with_all_fs
4040
from common import also_with_minimal_runtime, also_with_wasm_bigint, also_with_wasm64, flaky
4141
from common import EMTEST_BUILD_VERBOSE, PYTHON, WEBIDL_BINDER
4242
from common import requires_network, parameterize
@@ -13741,10 +13741,11 @@ def test_unistd_login(self):
1374113741
def test_unistd_sleep(self):
1374213742
self.do_run_in_out_file_test('unistd/sleep.c')
1374313743

13744-
@also_with_wasmfs
13744+
@crossplatform
13745+
@with_all_fs
1374513746
def test_unistd_fstatfs(self):
13746-
if not self.get_setting('WASMFS'):
13747-
self.skipTest("fstatfs is broken in js fs, will be fixed in PR #23381")
13747+
if '-DNODERAWFS' in self.emcc_args and WINDOWS:
13748+
self.skipTest('Cannot look up /dev/stdout on windows')
1374813749
self.do_run_in_out_file_test('unistd/fstatfs.c')
1374913750

1375013751
@no_windows("test is Linux-specific")

test/unistd/fstatfs.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,20 @@
22
#include <stdio.h>
33
#include <sys/vfs.h>
44
#include <unistd.h>
5+
#include <fcntl.h>
56

67
int main() {
78
struct statfs buf;
8-
int rtn = fstatfs(STDOUT_FILENO, &buf);
9-
assert(rtn == 0);
9+
10+
int rtn;
11+
assert(fstatfs(STDOUT_FILENO, &buf) == 0);
12+
printf("f_type: %ld\n", buf.f_type);
13+
14+
int f = open("file", O_RDWR | O_CREAT);
15+
assert(fstatfs(f, &buf) == 0);
16+
printf("f_type: %ld\n", buf.f_type);
17+
18+
assert(statfs("file", &buf) == 0);
1019
printf("f_type: %ld\n", buf.f_type);
1120
return 0;
1221
}

0 commit comments

Comments
 (0)