From 3d568fb7c948c1918bf8f3e95dd3709fd8fa8d61 Mon Sep 17 00:00:00 2001 From: RJ Rybarczyk Date: Mon, 26 Jul 2021 10:25:58 -0400 Subject: [PATCH] Sv2 ffi doc nits (#2) Minor interop-cpp ex doc syntax nits --- examples/interop-cpp/README.md | 124 +++++++++--------- .../no-serde-sv2/codec/src/codec/mod.rs | 4 +- protocols/v2/framing-sv2/src/framing2.rs | 8 +- 3 files changed, 66 insertions(+), 70 deletions(-) diff --git a/examples/interop-cpp/README.md b/examples/interop-cpp/README.md index 23222f6dc0c58..8f06aac01035f 100644 --- a/examples/interop-cpp/README.md +++ b/examples/interop-cpp/README.md @@ -1,14 +1,14 @@ # C++ interop -This crate provide an example of how to use the rust Sv2 `Decoder` and `Encoder` from C++. +This crate provide an example of how to use the Rust Sv2 `Decoder` and `Encoder` from C++. To run the example: `./run.sh`. -The example is composed by a rust "downstream node" that keep sending a +The example is composed by a Rust "downstream node" that keep sending a [`common_messages_sv2::SetupConnection`] message to a C++ "upstream node" that receive the message and keep answering with a [`common_messages_sv2::SetupConnectionError`]. -The rust codec is exported as a C static library by the crate [sv2-ffi](../../protocols/v2/sv2-ffi). +The Rust codec is exported as a C static library by the crate [sv2-ffi](../../protocols/v2/sv2-ffi). This crate also provide an [example](./template-provider/example-of-guix-build) of how to build the `sv2-ffi` as a static library using guix. @@ -21,7 +21,7 @@ The [header file](../../../protocols/v2/sv2-ffi/sv2.h) is generated with `cbindg Rust enums definition are transformed by `cbingen` in: ```c -struct [rust_enum_name] { +struct [Rust_enum_name] { union class Tag { [union_element_1_name] [union_element_2_name] @@ -51,8 +51,8 @@ struct [rust_enum_name] { } ``` -For example the below rust enum: -```rust +For example the below Rust enum: +```Rust #[repr(C)] pub enum CResult { Ok(T), @@ -88,13 +88,13 @@ struct CResult { ### Conventions #### Memory -All the memory used shared struct/enums (also when borrowed) is allocated by rust. +All the memory used shared struct/enums (also when borrowed) is allocated by Rust. When C++ take ownership of a Sv2 message the message must be manually dropped. #### Enums -In order to pattern match against a rust defined enum from C++: -```c +In order to pattern match against a Rust defined enum from C++: +``` CResult < CSv2Message, Sv2Error > frame = next_frame(decoder); switch (frame.tag) { @@ -112,29 +112,29 @@ case CResult < CSv2Message, Sv2Error > ::Tag::Err: }; ``` -### CVec -[`binary_sv2::binary_codec_sv2::CVec`] is used to share bytes buffers between rust and C++. +### `CVec` +[`binary_sv2::binary_codec_sv2::CVec`] is used to share bytes buffers between Rust and C++. A `CVec` can be either "borrowed" or "owned" if is on or the other depend by the method that we use to construct it. -* (borrowed) [`binary_sv2::binary_codec_sv2::CVec::as_shared_buffer`]: used when we need to fill a rust - allocated buffer from C++. This method do not guarantee anything about the pointed memory - and the user must enforce that the rust side do not free the pointed memory while the - C++ part is using it. - A CVec constructed with this method must not be freed by C++ (this is enforced by the fact that - the function used to free the CVec is not exported in the C library) -* (owned) `&[u8].into::()`: it copy the content of the `&[u8]` in a CVec. +* (borrowed) [`binary_sv2::binary_codec_sv2::CVec::as_shared_buffer`]: used when we need to fill a Rust + allocated buffer from C++. This method does not guarantee anything about the pointed memory + and the user must enforce that the Rust side does not free the pointed memory while the + C++ part is using it + A `CVec` constructed with this method must not be freed by C++ (this is enforced by the fact that + the function used to free the `CVec` is not exported in the C library) +* (owned) `&[u8].into::()`: used to copy the contents of the `&[u8]` to a `CVec`. It must be dropped from C++ via [`sv2_ffi::drop_sv2_message`] -* (owned) [`binary_sv2::binary_codec_sv2::cvec_from_buffer`]: used when a CVec must be created in C++, +* (owned) [`binary_sv2::binary_codec_sv2::cvec_from_buffer`]: used when a `CVec` must be created in C++, is used to construct a [`sv2_ffi::CSv2Message`] that will be dropped as usual with [`sv2_ffi::drop_sv2_message`] -* (owned) `CVec2.into::>()`, see CVec2 section +* (owned) `CVec2.into::>()`, see `CVec2` section * (owned) `Inner.into::()`: same as `&[u8].into::()` -### CVec2 -Is just a vector of CVec. Always allocated in rust. Is only used as field of Sv2 messages. It is -dropped when the Sv2 message get dropped. +### `CVec2` +A `CVec2` is a vector of `CVec`'s. It is always allocated in Rust, is used only as field of Sv2 messages, and is +dropped when the Sv2 message gets dropped. ## Memory management @@ -143,12 +143,12 @@ dropped when the Sv2 message get dropped. [`sv2_ffi::DecoderWrapper`] is instantiated in C++ via [`sv2_ffi::new_decoder`]. There is no need to drop it as it will live for the entire life of the program. -[`sv2_ffi::get_writable`] return a CVec. Is rust allocated memory that C++ can fill with the -socket content. The CVec is "borrowed" (`&[u8].into::()`) so it will be automatically -dropped by rust. +[`sv2_ffi::get_writable`] returns a `CVec` and is Rust allocated memory that C++ can fill with the +socket content. The `CVec` is "borrowed" (`&[u8].into::()`) so it will be automatically +dropped by Rust. -[`sv2_ffi::next_frame`] if a complete Sv2 frame is available it return a [`sv2_ffi::CSv2Message`] -the message could contain one or more "owned" CVec so it must be manually dropped via +[`sv2_ffi::next_frame`] is used if a complete Sv2 frame is available, it returns a [`sv2_ffi::CSv2Message`]. +The message can contain one or more "owned" `CVec`'s, so it must be manually dropped via [`sv2_ffi::drop_sv2_message`]. @@ -157,33 +157,32 @@ the message could contain one or more "owned" CVec so it must be manually droppe [`sv2_ffi::EncoderWrapper`] is instantiated in C++ via [`sv2_ffi::new_encoder`]. There is no need to drop it as it will live for the entire life of the program. -A [`sv2_ffi::CSv2Message`] can be constructed in C++ [here an example](./template-provider/template-provider.cpp#67) -if the message contains one ore more CVec the content of the CVec must be copied in a rust allocated -CVec with [`binary_sv2::binary_codec_sv2::cvec_from_buffer`]. The message must be dropped with -[`sv2_ffi::drop_sv2_message`] +A [`sv2_ffi::CSv2Message`] can be constructed in C++ ([here is an example](./template-provider/template-provider.cpp#67)) +if the message contains one or more `CVec`'s, then the content of the `CVec` must be copied in a Rust allocated +`CVec` with [`binary_sv2::binary_codec_sv2::cvec_from_buffer`]. The message must be dropped with +[`sv2_ffi::drop_sv2_message`]. -[`sv2_ffi::encode`] encode a [`sv2_ffi::CSv2Message`] as an encoded Sv2 frame in a buffer internal -to [`sv2_ffi::EncoderWrapper`]. The content of the buffer is returned as a "borrowed" CVec, after -that C++ do have copied it where needed it must free the encoder with [`sv2_ffi::free_encoder`], -this is necessary because the encoder will reuse the internal buffer to -encode the next message, with [`sv2_ffi::free_encoder`] we let the encoder know that the content of -the internal buffer has been copied and can be overwritten. +[`sv2_ffi::encode`] encodes a [`sv2_ffi::CSv2Message`] as an encoded Sv2 frame in a buffer internal +to [`sv2_ffi::EncoderWrapper`]. The buffer contents are returned as a "borrowed" `CVec`. After +that, C++ has copied it and it must free the encoder with [`sv2_ffi::free_encoder`]. +This is necessary because the encoder will reuse the internal buffer to encode the next message with +[`sv2_ffi::free_encoder`]. We let the encoder know that the content of the internal buffer has been copied +and can be overwritten. ## Decode Sv2 messages in C++ -1. instantiate a decoder with [`sv2_ffi::new_decoder`] -2. fill the decoder, copying the input bytes in the buffer returned by [`sv2_ffi::get_writable`] -3. if the above buffer is full call [`sv2_ffi::next_frame`], if the decoder has enough bytes to - decode an Sv2 frame it will return a `CSv2Message`, if not return to 2. +1. Instantiate a decoder with [`sv2_ffi::new_decoder`] +2. Fill the decoder, copying the input bytes in the buffer returned by [`sv2_ffi::get_writable`] +3. If the above buffer is full, call [`sv2_ffi::next_frame`]. If the decoder has enough bytes to + decode an Sv2 frame it will return a `CSv2Message`, otherwise it returns 2. ## Encode Sv2 messages in C++ -1. instantiate a encoder with [`sv2_ffi::new_encoder`] -2. call [`sv2_ffi::encode`] with a valid `CSv2Message` -3. copy the returned encoded frame where needed -4. call [`sv2_ffi::free_encoder`] in order to let the encoder know that the encoded frame has been - copied +1. Instantiate an encoder with [`sv2_ffi::new_encoder`] +2. Call [`sv2_ffi::encode`] with a valid `CSv2Message` +3. Copy the returned encoded frame where needed +4. Call [`sv2_ffi::free_encoder`] to let the encoder know that the encoded frame has been copied ## Build for C++ @@ -194,28 +193,25 @@ TODO An example of how to build a C++ program that use [`sv2_ffi`] can be found [here](./template-provider/example-of-guix-build/build.sh). -The build script do: -1. package `sv2_ffi` and all his dependency (this step wont be necessary as the packages will be -available in crates.io or github.com). -2. it call g++ in a guix container defined - [here](./template-provider/example-of-guix-build/example.scm) +The build script does the following: +1. Packages `sv2_ffi` and all its dependencies (this step wont be necessary as the packages will be +available in crates.io or github.com) +2. Calls g++ in a guix container defined [here](./template-provider/example-of-guix-build/example.scm) #### Manifest: -The first 255 lines of `example.scm` are just a copy paste of the -[guix cargo build system](`https://github.com/guix-mirror/guix/blob/13c4a377f5a2e1240790679f3d5643385b6d7635/guix/build-system/cargo.scm`) -the only thing that change is that it use rust-1.51 as it need const generics basic support (line 30 -of the manifest) +The first 255 lines of `example.scm` are a copy paste of the +[guix cargo build system](`https://github.com/guix-mirror/guix/blob/13c4a377f5a2e1240790679f3d5643385b6d7635/guix/build-system/cargo.scm`). +The only difference is that it uses Rust-1.51 as it needs `const` generics basic support (line 30 of the manifest) -At line 256 it begin the actual manifest: it build all the `sv2_ffi` dependency and then it build -`sv2_ffi`. +The actual manifest begins on L256, where it builds all the `sv2_ffi` dependencies and then builds `sv2_ffi`. -`sv2_ffi` is a library crate and the guix default behaviour is to not install rust library so the -install phase of `sv2_ffi` is replaced, and `sv2.h` and the new built `libsv2_ffi.a` are installed -in the container (they are installed in `/gnu/store/[hash]-rust-sv2_ffi-[version]/`) +`sv2_ffi` is a library crate and the guix default behavior is to not install the Rust library such that the +installation phase of `sv2_ffi` is replaced and `sv2.h` and the newly built `libsv2_ffi.a` are installed +in the container (they are installed in `/gnu/store/[hash]-Rust-sv2_ffi-[version]/`). -The manifest it expect to find `sv2.h` in the `sv2_ffi` package, as `sv2.h` is created manually with -`/build_header.sh` is very easy to commit code with a not updated header file, for that will be -added an action in github action to check if the header file is updated. +The manifest it expect to find `sv2.h` in the `sv2_ffi` package. Since the `sv2.h` is created manually with +`/build_header.sh`, it is very easy to commit code with an out of date header file. To ensure all commits include +the most updated header file, a GitHub Actions check is planned to be added. ## Install cbindgen diff --git a/protocols/v2/binary-sv2/no-serde-sv2/codec/src/codec/mod.rs b/protocols/v2/binary-sv2/no-serde-sv2/codec/src/codec/mod.rs index 287f5ec5f4dc2..91bef226d3f7f 100644 --- a/protocols/v2/binary-sv2/no-serde-sv2/codec/src/codec/mod.rs +++ b/protocols/v2/binary-sv2/no-serde-sv2/codec/src/codec/mod.rs @@ -9,13 +9,13 @@ pub mod decodable; pub mod encodable; mod impls; -/// Return the encoded byte size od a Decodable +/// Return the encoded byte size or a `Decodable` pub trait SizeHint { fn size_hint(data: &[u8], offset: usize) -> Result; fn size_hint_(&self, data: &[u8], offset: usize) -> Result; } -/// Return the encodec byte size of an Encodable coommprensive of the header if any +/// Return the encoded byte size of an `Encodable` comprehensive of the header, if any pub trait GetSize { fn get_size(&self) -> usize; } diff --git a/protocols/v2/framing-sv2/src/framing2.rs b/protocols/v2/framing-sv2/src/framing2.rs index bec3f9e8aa044..7331f4ea5fc69 100644 --- a/protocols/v2/framing-sv2/src/framing2.rs +++ b/protocols/v2/framing-sv2/src/framing2.rs @@ -149,7 +149,7 @@ impl<'a, T: Serialize + GetSize, B: AsMut<[u8]>> Frame<'a, T> for Sv2Frame } /// Try to build an Frame frame from a serializable payload. - /// It return a Frame if the size of the payload fit in the frame, if not it return None + /// It returns a Frame if the size of the payload fits in the frame, if not it returns None fn from_message(message: T, message_type: u8, extension_type: u16) -> Option { let len = message.get_size() as u32; Header::from_len(len, message_type, extension_type).map(|header| Self { @@ -230,9 +230,9 @@ impl<'a> Frame<'a, Vec> for NoiseFrame { self.payload.len() } - /// Try to build an Frame frame from a serializable payload. - /// It return a Frame if the size of the payload fit in the frame, if not it return None - /// Inneficient should be used only to build HandShakeFrames + /// Try to build a `Frame` frame from a serializable payload. + /// It returns a Frame if the size of the payload fits in the frame, if not it returns None + /// Inneficient should be used only to build `HandShakeFrames` fn from_message(message: Vec, _message_type: u8, _extension_type: u16) -> Option { if message.len() <= NOISE_MAX_LEN { let header = message.len() as u16;