-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add NODEFS readdir test case #15308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add NODEFS readdir test case #15308
Conversation
tests/test_core.py
Outdated
@@ -5274,6 +5274,10 @@ def test_fs_nodefs_home(self): | |||
self.emcc_args += ['-lnodefs.js'] | |||
self.do_runf(test_file('fs/test_nodefs_home.c'), 'success', js_engines=[config.NODE_JS]) | |||
|
|||
def test_nodefs_list_dir(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other tests here seem to prefix their name with test_fs_
@sbc100 I originally thought that I could create a simple fix for the listing of directories with NODEFS. Unfortunately my "fix" didn't fix anything for existing subfolders and my knowledge of emscripten FS is not good enough to provide a proper fix... The provided test case nevertheless shouldn't lead to a hard crash. |
The test case here was taken from #15308. The main bugfix here is to use `stream.node` rather than just `stream` when attempting to get node properties from a stream. There were some extra properties added to the streams back in #15167, but this change reverts that and instread fixes up the streams returned by NODERAWFS such that they have the correct/expected fields.
The test case here was taken from #15308. The main bugfix here is to use `stream.node` rather than just `stream` when attempting to get node properties from a stream. There were some extra properties added to the streams back in #15167, but this change reverts that and instread fixes up the streams returned by NODERAWFS such that they have the correct/expected fields.
The test case here was taken from #15308. The main bugfix here is to use `stream.node` rather than just `stream` when attempting to get node properties from a stream. There were some extra properties added to the streams back in #15167, but this change reverts that and instread fixes up the streams returned by NODERAWFS such that they have the correct/expected fields.
The test case here was taken from #15308. The main bugfix here is to use `stream.node` rather than just `stream` when attempting to get node properties from a stream. There were some extra properties added to the streams back in #15167, but this change reverts that and instread fixes up the streams returned by NODERAWFS such that they have the correct/expected fields.
The test case here was taken from #15308. The main bugfix here is to use `stream.node` rather than just `stream` when attempting to get node properties from a stream. There were some extra properties added to the streams back in #15167, but this change reverts that and instead fixes up the streams returned by NODERAWFS such that they have the correct/expected fields.
I just landed #16124 which I think fixes this issue, and uses the test case from this PR. So I think this can now be closes? |
Sounds good. Thanks for the fix! |
Commit #15167 introduced a bug in __syscall_getdents64.
Without this fix a simple dir listing of a mounted directory leads to a crash.