-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Make fstat work on file descriptor with no name #23058
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
Closed
hoodmane
wants to merge
40
commits into
emscripten-core:main
from
hoodmane:anonymous-file-descriptors
Closed
Changes from 4 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
f3f36d5
Make fstat work on file descriptor with no name
hoodmane 260278f
Fix node rawfs
hoodmane f865bdb
Cleanup
hoodmane cac8b19
Fix nodefs handling of unnamed file descriptors
hoodmane cad0ddf
Fix indentation
hoodmane 8b1711e
Apply review comments to test
hoodmane c961354
Fix flake8
hoodmane bebfc6b
Better argment names and comments
hoodmane 7f4533e
Fix ftruncate
hoodmane 2c41571
Fix directory fds for nameless file patch
hoodmane 6a4dfc2
Implement both truncate and ftruncate via truncateCommon
hoodmane 9d0c79a
Merge branch 'main' into anonymous-file-descriptors
hoodmane baadd84
Merge branch 'main' into anonymous-file-descriptors
hoodmane 3aa358f
Fix merge
hoodmane 04192f0
Merge branch 'main' into anonymous-file-descriptors
hoodmane 989cf22
Refactor things a bit more
hoodmane ad02402
Change doStat to writeStat
hoodmane 84bcf30
Fix tests
hoodmane ba35730
Fix reference error
hoodmane 4e67e7f
Fix test
hoodmane 9399402
Declare stream variable
hoodmane 7593dc1
Merge branch 'main' into anonymous-file-descriptors
hoodmane efff804
Merge branch 'main' into anonymous-file-descriptors
hoodmane 20de480
Merge branch 'main' into anonymous-file-descriptors
hoodmane e83ac75
Remove incorrect comment
hoodmane b509358
Fix merge
hoodmane 8c556c8
Address review comment
hoodmane 4ed8004
Merge branch 'main' into anonymous-file-descriptors
hoodmane bdfca83
Fix test
hoodmane 4fdc681
Fix test
hoodmane ddb3fab
Merge branch 'main' into anonymous-file-descriptors
hoodmane bfd07c7
Fix test
hoodmane 5d4c6d2
Merge branch 'main' into anonymous-file-descriptors
hoodmane ac100ab
Move stream ops access to streamGetAttr and streamSetAttr
hoodmane d1f1962
Fix syntax error
hoodmane 02e0b69
Slightly shorter
hoodmane 910b824
Slightly shorter
hoodmane 37f8ee9
Some fixes
hoodmane 5eef321
Fix more
hoodmane ad6f42a
More fixes
hoodmane File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| #include <fcntl.h> | ||
| #include <unistd.h> | ||
| #include <sys/stat.h> | ||
| #include <assert.h> | ||
| #include "stdio.h" | ||
|
|
||
| #ifdef __EMSCRIPTEN__ | ||
| #include <emscripten.h> | ||
| #endif | ||
|
|
||
| void makedir(const char *dir) { | ||
| int rtn = mkdir(dir, 0777); | ||
| assert(rtn == 0); | ||
| } | ||
|
|
||
| void changedir(const char *dir) { | ||
| int rtn = chdir(dir); | ||
| assert(rtn == 0); | ||
| } | ||
|
|
||
|
|
||
| int main() { | ||
| makedir("working"); | ||
hoodmane marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| #if defined(__EMSCRIPTEN__) && defined(NODEFS) | ||
| EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working')); | ||
| #endif | ||
| changedir("working"); | ||
hoodmane marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| int fd = open("file.txt", O_RDWR | O_CREAT, 0666); | ||
| unlink("file.txt"); | ||
| int res; | ||
| struct stat buf; | ||
| res = fstat(fd, &buf); | ||
| assert(res == 0); | ||
| assert(buf.st_atime > 1000000000); | ||
| res = fchmod(fd, 0777); | ||
| assert(res == 0); | ||
| res = ftruncate(fd, 10); | ||
| assert(res == 0); | ||
| printf("success\n"); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5921,6 +5921,21 @@ def test_fs_64bit(self): | |||||
| self.set_setting('FORCE_FILESYSTEM') | ||||||
| self.do_runf('fs/test_64bit.c', 'success') | ||||||
|
|
||||||
| @requires_node | ||||||
| @parameterized({ | ||||||
| '': ([],), | ||||||
| 'nodefs': (['-DNODEFS', '-lnodefs.js'],), | ||||||
| 'noderawfs':(['-sNODERAWFS'],), | ||||||
| }) | ||||||
hoodmane marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| def test_fs_stat_unnamed_file_descriptor(self, args): | ||||||
|
||||||
| def test_fs_stat_unnamed_file_descriptor(self, args): | |
| def test_fs_stat_unnamed_file_descriptor(self): |
hoodmane marked this conversation as resolved.
Show resolved
Hide resolved
hoodmane marked this conversation as resolved.
Show resolved
Hide resolved
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is this a common pattern where we try to do an operation first on the stream and then on the node if it fails?
Do we need the fallback?
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.
Well we have a map from file descriptors to streams, and a map from paths to nodes. Each stream has a reference to its node. The difficulty is that not every node has a valid path.
Most file systems store all relevant information about a node on the node itself. However, in the nodefs, each stream has a distinct native file descriptor. In the case where the node has no valid path, the only way to talk to the native object is with
fstatand friends using the native file descriptor. So in that case we need thesestream_ops. For every other FS, it's sufficient to havesetattron thenode_ops.Also, we do need the
node_opscase because if we callstatinstead offstatthen we have a path but no file descriptor. We couldopenthe path to get the file descriptor and thenstatit and close it. But we're better off just having a separate handler for gettattr/setattr on a path/node than on a file descriptor/stream. But only nodefs seems to need it.