-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Ensure the ino we give to readdir matches the ino we give to stat #23139
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
Merged
hoodmane
merged 19 commits into
emscripten-core:main
from
hoodmane:readdir_ino_matches_stat_ino
Dec 19, 2024
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
ae7bb24
Make sure the ino that we give to readdir matches the ino we give to …
hoodmane 9068d87
Some formatting fixes
hoodmane 61762e6
Revert
hoodmane 2d1a04c
Take two make nodefs inode numbers match
hoodmane 8981002
Fix make_dir_writable
hoodmane 57bd229
Remove subdir
hoodmane b5d5cb7
Merge branch 'main' into readdir_ino_matches_stat_ino
hoodmane c3f896e
Update
hoodmane 088b253
Apply review comments to test
hoodmane 7afc965
Add TODO comment about merging tests
hoodmane c499368
Clearer comment explaining chmod
hoodmane 6265dd6
effect => affect
hoodmane 11f1394
Fix test
hoodmane 7d12b2b
Merge branch 'main' into readdir_ino_matches_stat_ino
hoodmane 042f8ae
Add subdirectory again
hoodmane 6ed84c1
Update minimal_esm codesize
hoodmane 5742ac1
Merge branch 'main' into readdir_ino_matches_stat_ino
hoodmane ed7ea66
Merge branch 'main' into readdir_ino_matches_stat_ino
hoodmane 238a3bc
Don't do chmod in noderawfs
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
#include <stdio.h> | ||
#include <unistd.h> | ||
#include <assert.h> | ||
#include <fcntl.h> | ||
#include <string.h> | ||
#include <sys/stat.h> | ||
#include <assert.h> | ||
#include <errno.h> | ||
#include <string.h> | ||
#include <dirent.h> | ||
|
||
// TODO: Consider merging this with test/dirent/test_readdir.c? | ||
// That test doesn't work on nodefs, which has to be fixed before these two | ||
// tests can be merged . | ||
|
||
void setup() { | ||
// If the node working directory does not have execute permissions, then | ||
// `fs.readdir` will fail. For that reason we have to work in a subdirectory | ||
// and remove execute permissions from that. | ||
mkdir("sub", 0775); | ||
assert(chdir("sub") == 0); | ||
int res = open("b", O_CREAT, 0777); | ||
assert(res >= 0); | ||
assert(close(res) == 0); | ||
assert(symlink("b", "a") == 0); | ||
sbc100 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
int main() { | ||
setup(); | ||
int dirfd = open(".", O_RDONLY); | ||
assert(dirfd > 0); | ||
DIR *dirp = fdopendir(dirfd); | ||
assert(dirp != NULL); | ||
struct stat sta, stb; | ||
assert(lstat("a", &sta) == 0); | ||
assert(lstat("b", &stb) == 0); | ||
// Test that removing permissions from the directory does not affect the | ||
// already open directory handle that we have (dirp). | ||
#ifndef NODERAWFS | ||
assert(chmod(".", 0675) == 0); | ||
#endif | ||
int a_ino = -1; | ||
int b_ino = -1; | ||
struct dirent *ep; | ||
while ((ep = readdir(dirp))) { | ||
hoodmane marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (strcmp(ep->d_name, "a") == 0) { | ||
a_ino = ep->d_ino; | ||
} | ||
if (strcmp(ep->d_name, "b") == 0) { | ||
b_ino = ep->d_ino; | ||
} | ||
} | ||
assert(errno == 0); | ||
assert(a_ino >= 0); | ||
assert(b_ino >= 0); | ||
printf("readdir a_ino: %d, b_ino: %d\n", a_ino, b_ino); | ||
printf("stat a_ino: %llu, b_ino: %llu\n", sta.st_ino, stb.st_ino); | ||
assert(a_ino == sta.st_ino); | ||
assert(b_ino == stb.st_ino); | ||
printf("success\n"); | ||
return 0; | ||
} |
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
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.
I don't understand..
fs.readdir
should fail on any directory that doesn't have the execute bit set, regardless of weather that is CWD or a sub directory, right?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.
Apparently not.
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.
I suppose I can drop the chmod, but cding into a subdirectory does fix the test, at least when I run it locally. I don't really have a clear theory as to why since it seems a bit illogical.
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.
Ah, yes, I misremembered. The
x
bit is about traversing the heirachy, not list-ability. You can list directory than don't have thex
bit set. That is what ther
bit is for.