Skip to content
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

Fix logic errors in the zero-inode path. #352

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

sunfishcode
Copy link
Member

I've now tested the zero-inode path on a Wasm engine specially-modified to have fd_readdir set inode numbers to zero. Fix two bugs this turned up:

  • Increment buffer_processed, as noticed by @yamt
  • Don't do an fstatat on "..", because that references a path outside of the directory, which gets a permission-denied error.

I've now tested the zero-inode path on a Wasm engine specially-modified
to have `fd_readdir` set inode numbers to zero. Fix two bugs this turned up:
 - Increment `buffer_processed`, as noticed by @yamt
 - Don't do an `fstatat` on "..", because that references a path outside
   of the directory, which gets a permission-denied error.
Copy link
Contributor

@yamt yamt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sunfishcode sunfishcode merged commit 6cd1be1 into main Dec 6, 2022
@sunfishcode sunfishcode deleted the sunfishcode/readdir-fstatat branch December 6, 2022 17:16
john-sharratt pushed a commit to john-sharratt/wasix-libc that referenced this pull request Mar 6, 2023
I've now tested the zero-inode path on a Wasm engine specially-modified
to have `fd_readdir` set inode numbers to zero. Fix two bugs this turned up:
 - Increment `buffer_processed`, as noticed by @yamt
 - Don't do an `fstatat` on "..", because that references a path outside
   of the directory, which gets a permission-denied error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants