C/C++: add basic channel wrapper and log method #267
Conversation
| .to_str() | ||
| .expect("schema encoding is invalid"); | ||
| let data = std::slice::from_raw_parts(schema.data, schema.data_len); | ||
| foxglove::Schema::new(name, encoding, data) |
There was a problem hiding this comment.
This copies all the input, so you don't need to worry about the lifetime of passed C strings / data after the function returns (not sure if that was your question in the pr description)
There was a problem hiding this comment.
yeah, I think my concern was more about documentation, i.e. a user of the C/C++ library wouldn't necessarily know that it's ok for them to have a short lifetime if we don't describe it in comments
c/src/lib.rs
Outdated
| /// contained within a single allocated object. | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn foxglove_channel_log( | ||
| channel: Option<&mut FoxgloveChannel>, |
There was a problem hiding this comment.
I think this is the best way to handle pointers coming from C that may be null
There was a problem hiding this comment.
I was hoping to end up with a nonnull annotation on the C header side as well, but not sure cbindgen will allow me to do that while retaining the Option on the rust side. I wonder how receptive they are to upstream PRs...
| ); | ||
|
|
||
| private: | ||
| std::unique_ptr<foxglove_channel, void (*)(foxglove_channel*)> _impl; |
There was a problem hiding this comment.
nit: could also use a raw pointer and free it in the destructor
There was a problem hiding this comment.
what's the advantage there?
| REQUIRE(server.port() != 0); | ||
|
|
||
| foxglove::Channel channel{"example", "json", std::nullopt}; | ||
| const uint8_t data[] = {1, 2, 3}; |
There was a problem hiding this comment.
Does this not work?
| const uint8_t data[] = {1, 2, 3}; | |
| const std::byte data[] = {1, 2, 3}; |
There was a problem hiding this comment.
nope, integer literals can't be assigned without a cast, so I could do std::byte(1), ......
Changelog
C/C++: added Channel type.
Docs
None
Description
Resolves FG-10681
At least the basics of creating a schema+channel and logging messages seem to be working.
Some sadnesses/open areas to explore:
std::string_view) and a C options struct using C types (const char*). The conversion between these is super awkward. And the ownership/lifetime requirements of the string_views/pointers is not very clear (would need to just write it in comments).&T,&mut T, orNonNull<T>. However, reading a bit into how the Rust side treats those types, I'm not sure that checking them for null is really possible (the compiler might be allowed to assume they are not?) It seems likeOption<NonNull<T>>might work, but that's kinda nuts when you think about it (could it be working by accident?)Boxaround a type that is already internally anArc. Not sure there is any way around this though.std::span(C++20) over this ptr+length nonsense 😭