Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(mmap): support read-only #10782

Merged
merged 1 commit into from
Mar 28, 2020

Conversation

petersalomonsen
Copy link
Contributor

only do msync on munmap if PROT_WRITE is set

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Nice, thanks @petersalomonsen ! Just the one comment.

@@ -280,7 +280,9 @@ var SyscallsLibrary = {
if (len === info.len) {
#if FILESYSTEM && SYSCALLS_REQUIRE_FILESYSTEM
var stream = FS.getStream(info.fd);
SYSCALLS.doMsync(addr, stream, len, info.flags, info.offset);
if (info.prot & 0x02) {
Copy link
Member

Choose a reason for hiding this comment

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

I believe the constant can be {{{ cDefine('PROT_WRITE') }}}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for reviewing @kripken . Replaced with the constant now.

only do msync on munmap if PROT_WRITE is set
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Perfect, thanks @petersalomonsen !

@kripken kripken merged commit ea4b5cb into emscripten-core:master Mar 28, 2020
@petersalomonsen petersalomonsen deleted the fix-mmap-readonly branch March 29, 2020 05:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants