Serialize/Deserialize a std::ffi::CStr #30
Conversation
There was a problem hiding this comment.
@Michael-F-Bryan thanks for the PR, and sorry for the delay. This looks good to me. It might be good to add a pread/pwrite test as well ? Or perhaps a new section in the readme, examples, etc., up to you. And thanks again for this !
src/ctx.rs
Outdated
| }) | ||
| }; | ||
|
|
||
| let data = src.get(..null_byte+1).unwrap(); |
There was a problem hiding this comment.
Why not just src[..null_byte+1]?
Also so I’m too stupid right now to reason about the +1 indexes and since there are tests checking bytes read I’ll just assume it’s correct :P
There was a problem hiding this comment.
I remember hitting a really bizarre lifetimes issue, where &src[..null_byte + 1] would reborrow from src giving data a temporary lifetime which only lasted until the end of the function. The get() method returned an &[u8] with the 'a lifetime we want to return.
I believe I was writing this PR using nightly and when I tried to reproduce it in the playground everything worked 😞 Maybe there was a bug in NLL that's been fixed recently?
Also, the +1 is because .. is a half open range. When constructing the &CStr via CStr::from_bytes_with_nul_unchecked(data) we want to include that trailing null, so you need to specify the index after our null terminator.
Serialize/Deserialize a std::ffi::CStr
This PR adds implementations of
TryIntoCtxandTryFromCtxfor&CStrandCStringwhen compiling with thestdfeature enabled.We could probably speed the
TryFromCtxup a little bit by using something likememchrfor finding the string's null terminator seeing as that's the most expensive part, but that adds an extra dependency and I wasn't sure whether it was necessary.