Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust: Migrate to use ffi QUIC_API_TABLE #4756

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

youyuanwu
Copy link
Contributor

@youyuanwu youyuanwu commented Jan 20, 2025

Description

This is the beginning of a series of PRs to migrate from manually written code to use auto generated Rust ffi bindings, and make Rust APIs safe in default, and handles ffi internally in the crate.

This PR changes the following:

  • Migrate to use ffi QUIC_API_TABLE from manually written ApiTable.
  • Change Error::from_u32 and Error::ok to Error::ok_from_raw for dedicated error conversion from ffi function returns.
  • Reorganize apis like get_param, set_param and simplify other apis built on top of them.
  • Refactor the static api table initialization.
  • Add unified raw handle functions for all wrapper handle types. All wrappers support from_raw(HQUIC), into_raw()->HQUIC, as_raw()->HQUIC, close() and Drop. This allows user to use unsafe ffi apis directly.
  • Refactored the test code into test mod under test cfg section.
  • Removed types that are no longer needed or replaced by ffi mod.

The migration process will break the compatibility of the APIs but this is ok as msquic rust crate is experimental.

Testing

Existing rust test.
More tests will be added once we establish the Rust safe APIs.

Documentation

NA

@youyuanwu youyuanwu marked this pull request as ready for review January 21, 2025 02:09
@youyuanwu youyuanwu requested a review from a team as a code owner January 21, 2025 02:09
src/lib.rs Outdated Show resolved Hide resolved
@guhetier
Copy link

Thanks for following up on the error type! :)

@nibanks nibanks added the Language: Rust Related to the Rust interop layer label Jan 21, 2025
match event.event_type {
crate::STREAM_EVENT_START_COMPLETE => {
println!("Stream start complete 0x{:x}", unsafe {
event.payload.start_complete.status
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not specific to this PR, but why is accessing event.payload.start_complete.status considered unsafe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

payload is a union type. Rust union is for ffi only (without metadata), and the compiler has no way to check if the access is safe. However, Rust enum has metadata embedded in it so enum is safe. I have another branch that does the union to enum conversion (in unsafe code).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language: Rust Related to the Rust interop layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants