Skip to content

Comments

CON-382: Feedback on FFI conventions#10

Merged
alejnp merged 3 commits intorticommunity:masterfrom
alejnp:feature/CON-382-nonnull
Feb 19, 2026
Merged

CON-382: Feedback on FFI conventions#10
alejnp merged 3 commits intorticommunity:masterfrom
alejnp:feature/CON-382-nonnull

Conversation

@alejnp
Copy link
Member

@alejnp alejnp commented Feb 18, 2026

There are some idioms in Rust more suitable for our code, such as using std::ptr::NonNull.
We should prefer these to enforce a safer and cleaner code.

  • Native{} now holds a NonNull<T> instead of a *const T.
  • FFI calls that take a pointer "self" argument prefer NonNull<T>, because the C API expects non-null and NonNull<T> is transparent over *const T.

Additionally, we could use the chance to revisit the naming conventions on FFI-related types to better reflect their nature.

  • Native{} --> Ffi{}: "Native" could refer to anything, but FFI implies "glue".
  • {}Ptr --> Opaque{}: The types were never pointers to begin with.

@alejnp alejnp marked this pull request as ready for review February 19, 2026 11:50
@alejnp alejnp requested a review from Copilot February 19, 2026 11:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors FFI-related types and conventions to improve code safety and clarity in the Rust DDS Connector codebase. The changes adopt more idiomatic Rust patterns for FFI, specifically using std::ptr::NonNull<T> for non-null pointer guarantees and improving naming conventions to better reflect the nature of FFI types.

Changes:

  • Renamed wrapper types from Native{} prefix to Ffi{} prefix (e.g., NativeConnectorFfiConnector) to better indicate FFI glue code
  • Renamed opaque C types from {}Ptr suffix to Opaque{} prefix (e.g., ConnectorPtrOpaqueConnector) to clarify they are opaque types, not pointers
  • Changed FFI function parameters from raw pointers to NonNull<T> for non-null pointer parameters, providing compile-time safety guarantees

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/ffi/rtiddsconnector.rs Updated opaque type names and FFI function signatures to use NonNull<T> for non-null parameters; changed return type constness for consistency
src/ffi/mod.rs Renamed wrapper types from Native{} to Ffi{}, updated internal storage to use NonNull<T>, and adjusted usage patterns with NonNull::new()
src/connector.rs Updated references from NativeConnector to FfiConnector throughout

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alejnp alejnp merged commit 1c24552 into rticommunity:master Feb 19, 2026
4 checks passed
@alejnp alejnp deleted the feature/CON-382-nonnull branch February 19, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant