Change len from usize to u32.
#1944
Conversation
I don't think this is particularly important, but shouldn't harm either.
pepyakin
added
A2-insubstantial
| #[no_mangle] | ||
| pub extern "C" fn validate_block(params: *const u8, len: u32) -> u64 { | ||
| let params = unsafe { parachain::load_params(params, len) }; | ||
| let params = unsafe { parachain::load_params(params, len as u32) }; |
There was a problem hiding this comment.
I don't understand the motivation here if len is already u32 per the function definition.
There was a problem hiding this comment.
oh, I meant to cast it by usize 🙈 , I need to devote more attention to this PR.
|
Hold on guys, the third time is the charm |
bkchr
left a comment
bkchr
left a comment
There was a problem hiding this comment.
If you want to do this correctly, please change the `load_params' function ;)
Otherwise this pr doesn't bring any value.
|
Ok, I see your concern, had the same one. I chose the presented solution because the body of Also note, that this is not a destructive cast, since we never assume that the word size is below 32 bits. But if it were, then moving the cast won't help either, we would need to panic to somehow signal this destruction. And finally, I would have argued that this PR has some value, something along the lines that the risk of UB coming from a broken signature is worse than potential trimming problems in 16 bits platforms yadyadyadya (🙈), but I am not myself completely sure that the root problem is a sufficiently realistic scenario. So I am not sure if I am going to persue this PR. Feel free to close it. |
|
Oh, and look, the CI has passed xD |
|
Ok, Basti showed me another example where I am closing this, but if anybody feels motivated enough to pick up, feel free to reopen or open another PR |
I don't think this is particularly important, but shouldn't harm either.