-
Notifications
You must be signed in to change notification settings - Fork 980
Description
Summary
The QDP Rust code (qdp-core, qdp-python, pipeline_runner) uses unsafe in many places: raw-pointer encode APIs, DLPack lifecycle, CUDA FFI, and kernel launches. This issue proposes a structured audit, clearer safety boundaries, and optional safe wrappers so that unsafe is minimal, documented, and covered by tests.
Use Case
Maintainability: New contributors need to know where and why unsafe is used and what invariants they must uphold.
Safety: Reducing and isolating unsafe lowers the risk of misuse (e.g. dangling pointers, wrong device, double-free of DLPack).
Proposed Implementation
-
Reduce surface area
Public APIs: The encode-from-GPU-pointer APIs (encode_from_gpu_ptr, encode_from_gpu_ptr_f32, and with_stream / batch variants) are all unsafe fn taking raw pointers. Options:
Introduce a validated handle type (e.g. built from a device pointer + length + device id) that is constructed once with checks (e.g. validate_cuda_input_ptr) and then used in a safe encode_from(&self, handle, …) API so that callers only use unsafe at construction site.
Or keep unsafe fn but ensure all call sites (Rust tests, Python bindings) are documented and, where possible, wrapped behind a single module so the rest of the codebase stays safe.
DLPack: Consider a small RAII wrapper that holds *mut DLManagedTensor and calls the deleter in Drop, so that release_dlpack and manual unsafe { deleter(ptr) } are centralized and less error-prone.
Redundant unsafe: Remove inner unsafe where the function is already unsafe fn and the block adds no new contract (e.g. the inner block in encode_from_gpu_ptr_f32 that only forwards to encode_from_gpu_ptr_f32_with_stream). -
#![allow(unused_unsafe)]
The crate uses #![allow(unused_unsafe)] in several modules because CUDA vs non-CUDA builds differ (real unsafe kernel calls vs stubs). Keep the allow only where necessary and document in a short comment that “kernel calls are unsafe in CUDA builds; stubs are safe in no-CUDA builds.”
Alternatives Considered
Additional Context
https://doc.rust-lang.org/reference/unsafe-keyword.html
https://doc.rust-lang.org/nomicon/working-with-unsafe.html
https://rust-lang.github.io/unsafe-code-guidelines/introduction.html