Fix typos, bugs, and improve fallback mode compatibility#1
Merged
meshackbahati merged 2 commits intomainfrom Jun 25, 2025
Merged
Fix typos, bugs, and improve fallback mode compatibility#1meshackbahati merged 2 commits intomainfrom
meshackbahati merged 2 commits intomainfrom
Conversation
This commit addresses several issues identified during a codebase review: - Corrected typos in README.md and Cargo.toml (edition year). - Fixed a syntax error in a test within `netmap-min-sys/src/lib.rs`. - Enhanced `Frame` to use `Cow<'a, [u8]>`, allowing it to handle both borrowed (sys) and owned (fallback) data. This makes the `Frame` API more consistent across modes. - Updated `fallback::FallbackRxRing` to return `Option<Frame<'static>>` using owned data. - Added `fallback::create_fallback_channel` to facilitate testing of connected fallback rings. - Refactored `tests/mock.rs` to use `create_fallback_channel` and corrected test logic for ring capacity. Fallback tests now pass. - Adjusted `src/lib.rs` prelude for better conditional exports based on features. - Updated `Cargo.toml` to correctly associate `core_affinity` with `fallback` and `sys` features, and `reed-solomon-erasure` with the `sys` feature for relevant examples. - Gated `sys`-dependent examples (`ping_pong.rs`, `sliding_window_arq.rs`, `fec.rs`) with `#[cfg(feature = "sys")]` to prevent compilation errors when the `sys` feature is not active. - Modified `examples/thread_per_ring.rs` to operate conditionally based on the `sys` feature, allowing it to compile and run core pinning logic in fallback mode. - Addressed various clippy warnings, including unused imports, incorrect doc comment styles, and unnecessary mutable variables. Added `#[allow(unused_imports)]` for a `thiserror` macro that clippy flagged under specific fallback-only builds, believed to be a false positive. Note: Testing of the `sys` feature was not possible in the current sandboxed environment due to missing Netmap system dependencies. The changes aim to ensure the `fallback` mode is robust and well-tested.
This commit addresses several issues identified during a codebase review: - Corrected typos in README.md and Cargo.toml (edition year). - Fixed a syntax error in a test within `netmap-min-sys/src/lib.rs`. - Enhanced `Frame` to use `Cow<'a, [u8]>`, allowing it to handle both borrowed (sys) and owned (fallback) data. This makes the `Frame` API more consistent across modes. - Updated `fallback::FallbackRxRing` to return `Option<Frame<'static>>` using owned data. - Added `fallback::create_fallback_channel` to facilitate testing of connected fallback rings. - Refactored `tests/mock.rs` to use `create_fallback_channel` and corrected test logic for ring capacity. Fallback tests now pass. - Adjusted `src/lib.rs` prelude for better conditional exports based on features. - Updated `Cargo.toml` to correctly associate `core_affinity` with `fallback` and `sys` features, and `reed-solomon-erasure` with the `sys` feature for relevant examples. - Gated `sys`-dependent examples (`ping_pong.rs`, `sliding_window_arq.rs`, `fec.rs`) with `#[cfg(feature = "sys")]` to prevent compilation errors when the `sys` feature is not active. - Modified `examples/thread_per_ring.rs` to operate conditionally based on the `sys` feature, allowing it to compile and run core pinning logic in fallback mode. - Addressed various clippy warnings, including unused imports, incorrect doc comment styles, and unnecessary mutable variables. Added `#[allow(unused_imports)]` for a `thiserror` macro that clippy flagged under specific fallback-only builds, believed to be a false positive. Note: Testing of the `sys` feature was not possible in the current sandboxed environment due to missing Netmap system dependencies. The CI failure is related to the C-level Netmap kernel module compilation against an incompatible kernel version in the CI environment, not directly due to these Rust-level changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit addresses several issues identified during a codebase review:
netmap-min-sys/src/lib.rs.Frameto useCow<'a, [u8]>, allowing it to handle both borrowed (sys) and owned (fallback) data. This makes theFrameAPI more consistent across modes.fallback::FallbackRxRingto returnOption<Frame<'static>>using owned data.fallback::create_fallback_channelto facilitate testing of connected fallback rings.tests/mock.rsto usecreate_fallback_channeland corrected test logic for ring capacity. Fallback tests now pass.src/lib.rsprelude for better conditional exports based on features.Cargo.tomlto correctly associatecore_affinitywithfallbackandsysfeatures, andreed-solomon-erasurewith thesysfeature for relevant examples.sys-dependent examples (ping_pong.rs,sliding_window_arq.rs,fec.rs) with#[cfg(feature = "sys")]to prevent compilation errors when thesysfeature is not active.examples/thread_per_ring.rsto operate conditionally based on thesysfeature, allowing it to compile and run core pinning logic in fallback mode.#[allow(unused_imports)]for athiserrormacro that clippy flagged under specific fallback-only builds, believed to be a false positive.Note: Testing of the
sysfeature was not possible in the current sandboxed environment due to missing Netmap system dependencies. The changes aim to ensure thefallbackmode is robust and well-tested.