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 bufused when there are more entries to be read #176

Closed
wants to merge 3 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Sep 5, 2022

This commit updates the bufused value to be that of the buffer length
(buf_len) when there are more entries to be read.

Currently what is happening is that if another directory entry cannot
be written to the buffer then the current value of bufused is being
"returned", which indicates that all directory entries have been
written to the buffer which is not the case.

Fixes: #174


Using the provided reproducer in #174 running it produces:

$ ~/work/nodejs/node/node wasm-run.js tmp/ls.wasm tmp/testfolder/ | wc -l
301

This does not match the expected count of 300, but if we remove the pipe and the word count command and look at the output of the command we can see that there is an extra line in the output which will be included in the count.

0143
0248
0089
errno: 0

This commit updates the bufused value to be that of the buffer length
(buf_len) when there are more entries to be read.

Currently what is happening is that if another directory entry cannot
be written to the buffer then the current value of bufused is being
"returned", which indicates that all directory entries have been
written to the buffer which is not the case.

Fixes: nodejs#174
Copy link
Collaborator

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. I left one very small comment. Feel free to merge after addressing it.

test/test-fd-readdir-ls.c Outdated Show resolved Hide resolved
Remove commented out fprintf statement.
@danbev
Copy link
Contributor Author

danbev commented Sep 5, 2022

Landed in 92caba8.

@danbev danbev closed this Sep 5, 2022
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.

Directories with more than ~150 files are truncated
2 participants