-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[FS] Make fstat work on file descriptors with no name in memfs #23470
base: main
Are you sure you want to change the base?
[FS] Make fstat work on file descriptors with no name in memfs #23470
Conversation
This makes fstat work on anonymous memfs file descriptors. It is split off from emscripten-core#23058.
So code size went down by a little. But now |
Well it's always possible to directly copy the ci output. Maybe check_expectations could print something like the following when it fails so I could paste it into the terminal and apply the updates that CI wanted:
|
I'm hoping to have the CI automatically add make those change perhaps? In order to get the same results as the CI you need to make sure you do |
I think I have been doing that, but it seems to work only some of the time... |
Oh, there is also the case when This happens when llvm of binaryen changes effect code sizes. In that case I normally push a specific update commit, which I can look into doing now. |
Indeed if you looks at the failing CI test its called I updated main in 12ca58d |
Okay I rearranged the code in attempt to address your comments. Does it look better? |
ftruncate(fd, len) { | ||
var stream = FS.getStreamChecked(fd); | ||
if ((stream.flags & {{{ cDefs.O_ACCMODE }}}) === {{{ cDefs.O_RDONLY}}}) { | ||
if (len < 0 || (stream.flags & {{{ cDefs.O_ACCMODE }}}) === {{{ cDefs.O_RDONLY}}}) { |
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 separate fix?
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.
Yeah
This makes fstat work on anonymous memfs file descriptors. It is split off from #23058.