Allow compilation on 32 bit architectures #213
Conversation
|
I think I followed the contribution guidelines, but please let me know if this is missing anything! |
nashley
force-pushed
the
allow-compilation-on-32-bit-architectures
branch
2 times, most recently
from
b257d4f
to
b3e690a
Compare
|
rust-lang/rust#70460 would allow this to be done more cleanly than only allowing compilation on platforms where we know the |
src/wire_msg.rs
Outdated
| { | ||
| compile_error!("You need an architecture capable of addressing 32 bit pointers"); | ||
| } | ||
| let mut data: Vec<u8> = vec![0; usize::try_from(msg_header.data_len()).unwrap()]; |
There was a problem hiding this comment.
We try to avoid using .unwrap() wherever we can to prevent panics.
There was a problem hiding this comment.
Thanks for taking a look!
This should be an infallible operation on 32 and 64 bit platforms, which the preceding lines should ensure. The only built-in compile-time alternative I know of is to wait on rust-lang/rust#70460, but if you know of another way, I'd love to hear it.
There was a problem hiding this comment.
Thanks for the PR, nice one. Where the type is a result then ? is OK here.
Where we see a result we always use it or pass an error back (or resolve the error).
There was a problem hiding this comment.
I added in a runtime check as well. Do we want to keep both the compile-time check and the (theoretically unreachable) runtime check?
nashley
force-pushed
the
allow-compilation-on-32-bit-architectures
branch
from
2eb41f6
to
1352568
Compare
nashley
force-pushed
the
allow-compilation-on-32-bit-architectures
branch
2 times, most recently
from
2790662
to
2cce2eb
Compare
nashley
force-pushed
the
allow-compilation-on-32-bit-architectures
branch
from
2cce2eb
to
ca9b97d
Compare
|
I'd love this to be merged. |
- Don't assume that usize is equivalant to a u64 - Require 32-bit or 64-bit architecture at compile time BREAKING CHANGE: this changes the data_len field in MsgHeader from usize to u32
lionel-faber
force-pushed
the
allow-compilation-on-32-bit-architectures
branch
from
ca9b97d
to
67ab0a2
Compare
|
I've rebased this and updated the commit message mentioning |
Don't assume that usize is equivalant to a u64