Add safe Memory::read and Memory::write methods#2528
Conversation
theduke
force-pushed
the
issue-2484-safe-memory-access
branch
from
ffceb5b to
7fac8d1
Compare
github-actions
Bot
added
the
wasmtime:api
Subscribe to Label Actioncc @peterhuene DetailsThis issue or pull request has been labeled: "wasmtime:api"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
|
|
||
| /// Copies memory contents at the given offset into a buffer. | ||
| /// | ||
| /// Returns an error if the copy failed. |
There was a problem hiding this comment.
Could this documentation detail the error cases as well? It also might be good to mention that the size of the copy is the size of the buffer itself.
There was a problem hiding this comment.
I expanded the docs for both read and write a bit.
theduke
force-pushed
the
issue-2484-safe-memory-access
branch
3 times, most recently
from
8aea7ed to
3d10b20
Compare
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
I think there may be some rustfmt issues too?
|
I implemented your suggestion, the code definitely is more elegant this way. I also added a |
theduke
force-pushed
the
issue-2484-safe-memory-access
branch
from
3d10b20 to
74fbb3c
Compare
This commit introduces two new methods on `Memory` that enable reading and writing memory contents without requiring `unsafe`. The methods return a new `MemoryError` if the memory access fails.
theduke
force-pushed
the
issue-2484-safe-memory-access
branch
from
74fbb3c to
e0e2bc1
Compare
|
Looks great to me, thanks! |
2 participants
Introduce
Memory::readandMemory::writemethods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memorydocs.Fixes #2484.
Currently most fallible methods use
anyhow::Error.I have instead added a new
MemoryError, since recovering from such errors is possible and it's more expressive.(future proofed with
#[non_exhaustive]).Could to change it to a
anyhow::Errorif that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Lockedvariant, which is not required yet, but apparently will be once threading support lands. Happy to remove it or slap on a#[doc(hidden)]to prevent confusion.