move mmap into sysdeps by froydnj · Pull Request #524 · bytecodealliance/lucet · GitHub
Skip to content
This repository was archived by the owner on Mar 24, 2022. It is now read-only.

move mmap into sysdeps - #524

Open
froydnj wants to merge 1 commit into
bytecodealliance:mainfrom
froydnj:move-mmap-to-sysdeps
Open

froydnj wants to merge 1 commit into
bytecodealliance:mainfrom
froydnj:move-mmap-to-sysdeps

Conversation

@froydnj

@froydnj froydnj commented May 15, 2020

Copy link
Copy Markdown
Contributor

The patch is relatively easy; what's not clear to me is how something
like this really wants to be exported. Should sysdeps export a
MappedRegion that would be backed by VirtualAlloc on Windows?
Should sysdeps just export everything with their platform names and
let lucet-runtime use conditionals to choose what to export?

This patch punts on those questions for now; we're just trying to move
the obvious system-dependent code into sysdeps.

The patch is relatively easy; what's not clear to me is how something
like this really wants to be exported.  Should `sysdeps` export a
`MappedRegion` that would be backed by `VirtualAlloc` on Windows?
Should `sysdeps` just export everything with their platform names and
let `lucet-runtime` use conditionals to choose what to export?

This patch punts on those questions for now; we're just trying to move
the obvious system-dependent code into `sysdeps`.
@froydnj
froydnj requested a review from acfoltzer May 18, 2020 14:12
@acfoltzer
acfoltzer requested review from fst-crenshaw and removed request for acfoltzer May 21, 2020 19:11
@fst-crenshaw

Copy link
Copy Markdown
Contributor

@pchickey pchickey closed this Jun 26, 2020
@pchickey

Copy link
Copy Markdown
Contributor

@acfoltzer acfoltzer reopened this Jun 26, 2020
@acfoltzer
acfoltzer changed the base branch from master to main June 26, 2020 00:51

This branch has not been deployed

No deployments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants