wasi: resets offsets in reopen #1866
Merged
Conversation
This file contains 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
yagehu
requested review from
codefromthecrypt,
mathetake and
evacchi
as code owners
evacchi
reviewed
Dec 12, 2023
yagehu
force-pushed
the
yagehu/append-offset
branch
2 times, most recently
from
f8af47b
to
4ad7124
Compare
Fixed! |
evacchi
reviewed
Dec 12, 2023
evacchi
approved these changes
Dec 12, 2023
This commit fixes `fd_fdstat_set_flags` resetting the fd offset due to it reopening the file every time. This fix gets the current offset before reopening and restore the offset with seek. Signed-off-by: Yage Hu <me@huyage.dev>
|
thanks! |
yagehu
force-pushed
the
yagehu/append-offset
branch
from
4ad7124
to
7478b88
Compare
|
I see you force-pushed new changes, let me know when the PR is in a stable state @yagehu ! :) |
I'm done. Just a quick fix to avoid double |
evacchi
approved these changes
Dec 12, 2023
mathetake
changed the title
fd_fdstat_set_flags resetting offset
ncruces
reviewed
Dec 13, 2023
| } | ||
|
|
||
| if !isDir { | ||
| offset, err = f.file.Seek(0, 1) |
There was a problem hiding this comment.
Maybe too late for a nit, but please constants for the whence: io.SeekStart, etc…
There was a problem hiding this comment.
Good point, let's fix it in a follow up PR
gaukas
pushed a commit
to refraction-networking/wazero
that referenced
this pull request
Dec 14, 2023
Signed-off-by: Yage Hu <me@huyage.dev>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This commit fixes
fd_fdstat_set_flagsresetting the fd offset due to it reopening the file every time. This fix gets the current offset before reopening and restore the offset with seek. This addresses #1865.NB: A couple of existing tests are actually spec'd wrong. To fix, I introduced addition
Seekcalls before checking removing append flag.