impls for null-terminated FFI string types

[jonhoo]

From #800: serde currently ships with an impl of Serialize and Deserialize for str and String, but not for the null-terminated C-like string types ffi::CStr and ffi::CString. Under the hood, these string implementations are very similar, and providing serialization implementations for the latter two shouldn't be too hard.

This patch adds serialization and deserialization of those types through null-terminated [u8]s. I'm guessing I should also implement tests for this, but didn't find where those should go. Feedback welcome!

[dtolnay]

Please remove this impl unless you can implement it safely. The internal representation of CStr is not stable and there are plans to change it in the future.

[jonhoo]

Ah, that's unfortunate. This is basically a direct translation of String::into_boxed_str, since the equivalent CString::into_boxed_cstr method does not exist. I think it's more common (and useful) to have the impl for CStr... Also allows working with, say Arc<CStr> (or arccstr). Should I then also remove the Serialize impl?

[dtolnay]

No, keep the Serialize impl. It's fine to support serializing a type without supporting deserializing it.

[dtolnay]

An empty vec would be an error right?

[dtolnay]

This does not require parentheses around self.to_bytes_with_nul().

[dtolnay]

What are the reasons for including the null terminator in the serialized representation?

[dtolnay]

Using serialize_bytes would allow most serializers to serialize this more efficiently than other types of slices.

[jonhoo]

Good point about including the NULL bytes in the serialization. I'll remove that.

[dtolnay]

Tests for serialization should go here, tests for deserialization should go here, and tests for various deserialization error cases should go here.

[jonhoo]

Ah, so in writing the tests I realized that using serialize_bytes means we also need a way of turning the decoded Box<[u8]> into Vec<u8>. Unfortunately, there is no inverse function of Vec::into_boxed_slice, so there isn't an obvious way of doing that without doing an extra copy :/

[jonhoo]

Unless there's a way to deserialize_bytes into Vec<u8> instead of a Box<[u8]>.

[dtolnay]

ByteBuf implements Deserialize and Into<Vec<u8>>. Would that help? Something like:

let v: Vec<u8> = try!(ByteBuf::deserialize(deserializer)).into();

[jonhoo]

That works, though I'd have to import ByteBuf into de/impl.rs?

[dtolnay]

Yep that's fine.

[jonhoo]

So, it looks like we can also land deserialization for CStr once into_boxed_c_str lands (it is currently nightly-only). May be worth doing that in a separate PR though.

[jonhoo]

@dtolnay hmm -- why is bytes::ByteBuf only available on beta?

[dtolnay]

You're missing #[cfg(feature = "std")] on the import.

[jonhoo]

Okay, I think this is ready to merge @dtolnay. Deserialize for CStr can come later when into_boxed_c_str stabilizes.

[clarfonthey]

@jonhoo @dtolnay is there something open on serde's end to implement that once the method is stabilised? there's now a tracking issue for that feature at rust-lang/rust#40380

[jonhoo]

@clarcharr Not as far as I know, but I'm watching the issue and the PR, so will submit a follow-up PR here once it lands.

[dtolnay]

Thanks @clarcharr and @jonhoo, I filed #810 to track this on our end.