refactor(io): accept multiple buffer types in AncillaryBuilder#795
refactor(io): accept multiple buffer types in AncillaryBuilder#795Berrysoft wants to merge 11 commits intocompio-rs:masterfrom
Conversation
|
How do you ensure alignment? |
|
Our |
|
I admit that there might be unsound in this PR. |
Correct |
|
😢 We need to refactor the inner |
There was a problem hiding this comment.
Pull request overview
Refactors ancillary (control message) construction to allow AncillaryBuilder to operate over arbitrary IoBufMut buffers (including custom DST buffers), and removes the now-obsolete deprecated CMsg* compatibility types.
Changes:
- Generalize
AncillaryBuilderfromAncillaryBuf<N>toIoBufMutand remove deprecatedCMsgRef/CMsgIter/CMsgBuildershims. - Remove deprecated
compio-netre-exports ofCMsg*types. - Update/add tests, including coverage for a custom DST buffer backed by
[MaybeUninit<u8>].
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| compio-net/src/lib.rs | Removes deprecated CMsg* type aliases now that compio_io::ancillary is the intended API. |
| compio-io/tests/ancillary.rs | Updates tests to use AncillaryBuilder and adds a custom DST buffer test for [MaybeUninit<u8>]. |
| compio-io/src/ancillary/windows.rs | Removes CMsg* helper methods only used by the deprecated compatibility layer. |
| compio-io/src/ancillary/unix.rs | Removes CMsg* helper methods only used by the deprecated compatibility layer. |
| compio-io/src/ancillary/mod.rs | Generalizes AncillaryBuilder to IoBufMut and deletes deprecated CMsg* wrapper types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1ae3506 to
cac3224
Compare
|
The inner |
| } | ||
| } | ||
|
|
||
| pub(crate) struct CMsgIter { |
There was a problem hiding this comment.
Maybe pub(super)? Given that this impl highly depends on the caller feeding back the same pointer, we may want to limit its scope to only the super mod.rs (and siblings perhaps in the future)?
There was a problem hiding this comment.
The super is declaring mod sys without pub(crate) so I think it's fine:)
Previously, I agreed to keep the deprecated types because there's no replacement for
[MaybeUninit<u8>]. Now I refactoredAncillaryBuilderto accept more buffer types, and have added a test for[MaybeUninit<u8>]. We can gracefully remove theCMsgBuilderthen.