[pointer] Ptr::try_cast_into returns two Ptrs #1161
Conversation
joshlf
force-pushed
the
ptr-split
branch
4 times, most recently
from
4be4755 to
43761b0
Compare
joshlf
force-pushed
the
ptr-split
branch
8 times, most recently
from
18e3b66 to
541396d
Compare
| // `ptr` is non-null. | ||
| let ptr = unsafe { NonNull::new_unchecked(ptr) }; | ||
|
|
||
| // SAFETY: TODO(#896) |
There was a problem hiding this comment.
NOTE: Leaving this undone, but it will block 0.8.
src/lib.rs
Outdated
| // PANICS: This is guaranteed not to underflow (and thus panic) since | ||
| // `remainder` addresses subset of `bytes`, and thus can't be longer | ||
| // than `bytes`. | ||
| let split_at = bytes.len().checked_sub(remainder.len()).expect("zerocopy internal error: `bytes.len().checked_sub(remainder.len())` should not underflow"); |
There was a problem hiding this comment.
Heads up about this .expect(). If we go the "always return an error instead of panicking" route, even for errors we know are unreachable, then it's not clear to me what error to construct here (it was easier when we were returning Options). I could also be convinced to do unchecked_sub.
There was a problem hiding this comment.
Since it's unreachable, it doesn't matter what error we'd return (though I'd argue that a SizeError is most appropriate). But let's just do an unchecked_sub here!
jswrenn
left a comment
jswrenn
left a comment
There was a problem hiding this comment.
Giving this an approval, but there's one suggestion in my review.
src/lib.rs
Outdated
| // PANICS: This is guaranteed not to underflow (and thus panic) since | ||
| // `remainder` addresses subset of `bytes`, and thus can't be longer | ||
| // than `bytes`. | ||
| let split_at = bytes.len().checked_sub(remainder.len()).expect("zerocopy internal error: `bytes.len().checked_sub(remainder.len())` should not underflow"); |
There was a problem hiding this comment.
Since it's unreachable, it doesn't matter what error we'd return (though I'd argue that a SizeError is most appropriate). But let's just do an unchecked_sub here!
The first return value is unchanged - it is the target of the cast. The second return value replaces the previous `split_at: usize`, and replaces it with a pointer to the prefix or suffix bytes as a `Ptr<[u8]>`. This permits us to replace uses of `Ref` with direct uses of `Ptr` in some places. Co-authored-by: Jack Wrenn <[email protected]>
2 participants
The first return value is unchanged - it is the target of the cast. The second return value replaces the previous
split_at: usize, and replaces it with a pointer to the prefix or suffix bytes as aPtr<[u8]>.This permits us to replace uses of
Refwith direct uses ofPtrin some places.