From ce07ed1c07981df7ed6d5a069165cb8e1343feea Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 17 Dec 2024 09:28:18 +0100 Subject: [PATCH] fix: Mark all StorePath pointer management as unsafe ... because it is. I had previously dismissed the comparatively trivial unsafety of these functions, assuming the caller is aware of the purpose of them and reasonably familiar with manual memory management. That would have been fine in an unsafe by default language like C++, which Rust is not. (cherry picked from commit b43455fdd0468f067741a79a7031ba2fa907f0eb) --- rust/nix-store/src/path.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/rust/nix-store/src/path.rs b/rust/nix-store/src/path.rs index 5fa847c..fc20382 100644 --- a/rust/nix-store/src/path.rs +++ b/rust/nix-store/src/path.rs @@ -14,18 +14,27 @@ impl StorePath { /// This is a low level function that you shouldn't have to call unless you are developing the Nix bindings. /// /// Construct a new `StorePath` by first cloning the C store path. - /// This does not take ownership of the C store path, so it should be a borrowed value, or you should free it. - pub fn new_raw_clone(raw: NonNull) -> Self { + /// + /// # Safety + /// + /// This does not take ownership of the C store path, so it should be a borrowed pointer, or you should free it. + pub unsafe fn new_raw_clone(raw: NonNull) -> Self { Self::new_raw( - NonNull::new(unsafe { raw::store_path_clone(raw.as_ptr()) }) + NonNull::new(raw::store_path_clone(raw.as_ptr())) .or_else(|| panic!("nix_store_path_clone returned a null pointer")) .unwrap(), ) } + /// This is a low level function that you shouldn't have to call unless you are developing the Nix bindings. /// /// Takes ownership of a C `nix_store_path`. It will be freed when the `StorePath` is dropped. - pub fn new_raw(raw: NonNull) -> Self { + /// + /// # Safety + /// + /// The caller must ensure that the provided `NonNull` is valid and that the ownership + /// semantics are correctly followed. The `raw` pointer must not be used after being passed to this function. + pub unsafe fn new_raw(raw: NonNull) -> Self { StorePath { raw } } pub fn name(&self) -> Result { @@ -40,7 +49,14 @@ impl StorePath { } } - pub fn as_ptr(&self) -> *mut nix_c_raw::StorePath { + /// This is a low level function that you shouldn't have to call unless you are developing the Nix bindings. + /// + /// Get a pointer to the underlying Nix C API store path. + /// + /// # Safety + /// + /// This function is unsafe because it returns a raw pointer. The caller must ensure that the pointer is not used beyond the lifetime of this `StorePath`. + pub unsafe fn as_ptr(&self) -> *mut nix_c_raw::StorePath { self.raw.as_ptr() } }