theduke opened PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, which is not required yet, but apparently will be once threading support lands. Happy to remove it or slap on a#[doc(hidden)]
.
theduke edited PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, 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.
theduke updated PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, 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.
alexcrichton submitted PR Review.
alexcrichton created PR Review 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.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
FWIW I don't think that we'll ever need this error because we'll basically be implementing
memory.copy
semantics which would never need an error for this.
alexcrichton created PR Review Comment:
I think here this will want to use
checked_add
to avoid overflow.
theduke submitted PR Review.
theduke created PR Review Comment:
In that case ... should the error just be a struct?
I don't see any other relevant error conditions.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
That's true yeah, and I think we could go ahead and have it just be an opaque structure with a singular private field so we can add more to it later if we want.
theduke updated PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, 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.
theduke submitted PR Review.
theduke created PR Review Comment:
Done.
theduke created PR Review Comment:
Ah yeah, I'm too used to
debug
semantics for overflow checking...Both
read
andwrite
now usechecked_add
.
theduke submitted PR Review.
theduke submitted PR Review.
theduke created PR Review Comment:
I expanded the docs for both
read
andwrite
a bit.
theduke updated PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, 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.
theduke updated PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, 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.
theduke updated PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, 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.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I think it should be ok to drop the
PartialEq
here
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
It's ok to remove this comment, I don't think it adds much over what the existence of the
checked_add
already implies.
alexcrichton created PR Review Comment:
Although thinking about this again, another perhaps safer way to write these methods would be:
let src = self.data_unchecked().get(offset..offset + buffer.len())?; buffer.copy_from_slice(src); Ok(())
(and something similar below)
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Or actually to avoid having to do checked arithmetic:
let src = self.data_unchecked().get(offset..).get(..buffer.len())?;
theduke updated PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, 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.
theduke updated PR #2528 from issue-2484-safe-memory-access
to main
:
Introduce
Memory::read
andMemory::write
methods for accessing memory contents safely.Also adds a test and mentions the the new methods in the
Memory
docs.Fixes #2484.
Currently most fallible methods use
anyhow::Error
.
I have instead added a newMemoryError
, since recovering from such errors is possible and it's more expressive.
(future proofed with#[non_exhaustive]
).Could to change it to a
anyhow::Error
if that's the preferred strategy (or wrap the MemoryError with anyhow).I also already added a
MemoryError::Locked
variant, 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.
alexcrichton merged PR #2528.
Last updated: Dec 23 2024 at 12:05 UTC