Avoid std::ptr::addr_of! in BufRingEntry::tail to support stable Debian #161
Conversation
| @@ -264,6 +264,6 @@ impl BufRingEntry { | |||
| /// a valid pointer type, but also the argument must be the address to the first entry | |||
| /// of the buf_ring for the resv field to even be considered the tail field of the ring. | |||
| pub unsafe fn tail(ring_base: *const BufRingEntry) -> *const u16 { | |||
| std::ptr::addr_of!((*ring_base).0.resv) | |||
| &(*ring_base).0.resv | |||
There was a problem hiding this comment.
Is this unsound because it may create a valid reference to uninitialized memory?
There was a problem hiding this comment.
We know that the memory is zero-initialized. I don't know if that is enough to avoid UB according to the language's rules, though. We could get around that question by manually initializing the first BufRingEntry in the ring, in InnerBufRing::new.
In that case, do we want to do without this tail method completely?
There was a problem hiding this comment.
Maybe we should use autocfg so that we can use addr_of! on new compiler.
There was a problem hiding this comment.
I'm not sure, we'd still have to make sure that the code compatible with older compilers is correct, but then there isn't much point in having another variant of the same code doing the same thing.
There was a problem hiding this comment.
I believe that on old compiler it was probably more of a definition UB and didn't really cause problems. The point of using addr_of! is that old code will work if compiler behavior changes in future.
like https://github.com/Gilnaa/memoffset/blob/master/src/raw_field.rs#L21
I've thought about it, and a C repr type reference to a zero-fill buffer should be safe. but not using addr_of! means a stricter requirement on parameter.
There was a problem hiding this comment.
I believe that on old compiler it was probably more of a definition UB and didn't really cause problems. The point of using
addr_of!is that old code will work if compiler behavior changes in future.
Ah, I see.
I've thought about it, and a C repr type reference to a zero-fill buffer should be safe. but not using
addr_of!means a stricter requirement on parameter.
Is BufRingEntry::tail really useful to external users? If not, we could just drop it, given it is unstable.
There was a problem hiding this comment.
Is
BufRingEntry::tailreally useful to external users? If not, we could just drop it, given it is unstable.
?
Debian bullseye (the current stable release) is still on Rust 1.48. Thankfully, (*ring_base).0.resv will always be 2-byte aligned, so we can just get a reference to it and coerce that into a pointer, assuming the field is properly initialized. Signed-off-by: Alberto Faria <afaria@redhat.com>
|
v2:
|
|
Thank you! |
Debian bullseye (the current stable release) is still on Rust 1.48. Thankfully,
(*ring_base).0.resvwill always be 2-byte aligned, so we can just get a reference to it and coerce that into a pointer.