Skip to content

Add lifetimes to `ConstYDBBuffer` with PhantomData

Right now, using the buffer after it's freed will not be a compile error. That doesn't happen because ConstYDBBuffer is private and only used in one or two places, but it would be nice to enforce that it can't be used after a free. The following patch adds lifetimes to it:

diff --git a/src/simple_api/mod.rs b/src/simple_api/mod.rs
index b52ccbf..f5579e6 100644
--- a/src/simple_api/mod.rs
+++ b/src/simple_api/mod.rs
@@ -66,7 +66,7 @@
 
 pub mod call_in;
 
-use std::error::Error;
+use std::{error::Error, marker::PhantomData};
 use std::ops::{Deref, DerefMut, Index, IndexMut};
 use std::ptr;
 use std::ffi::CString;
@@ -1230,30 +1230,30 @@ impl Key {
 #[repr(transparent)]
 /// Because of this repr(transparent), it is safe to turn a
 /// `*const ConstYDBBuffer` to `*const ydb_buffer_t`
-struct ConstYDBBuffer(ydb_buffer_t);
+struct ConstYDBBuffer<'a>(ydb_buffer_t, PhantomData<&'a [u8]>);
 
-impl ConstYDBBuffer {
+impl ConstYDBBuffer<'_> {
     fn as_ptr(&self) -> *const ydb_buffer_t {
         &self.0
     }
 }
 
-impl From<ydb_buffer_t> for ConstYDBBuffer {
+impl From<ydb_buffer_t> for ConstYDBBuffer<'static> {
     fn from(buf: ydb_buffer_t) -> Self {
-        Self(buf)
+        Self(buf, PhantomData)
     }
 }
 
-// TODO: this is unsafe and could allow using the slice after it goes out of scope
-// TODO: this is ok for now because `ConstYDBBuffer` is only used locally within a single function.
-// TODO: possible fix: use `PhantomData` for `ConstYDBBuffer`
-impl From<&[u8]> for ConstYDBBuffer {
+impl<'a> From<&'a [u8]> for ConstYDBBuffer<'a> {
     fn from(slice: &[u8]) -> Self {
-        Self(ydb_buffer_t {
-            buf_addr: slice.as_ptr() as *mut _,
-            len_used: slice.len() as u32,
-            len_alloc: slice.len() as u32,
-        })
+        Self(
+            ydb_buffer_t {
+                buf_addr: slice.as_ptr() as *mut _,
+                len_used: slice.len() as u32,
+                len_alloc: slice.len() as u32,
+            },
+            PhantomData,
+        )
     }
 }
 

Unfortunately that causes a compile error:

error[E0502]: cannot borrow `self.subscripts` as mutable because it is also borrowed as immutable
    --> src/simple_api/mod.rs:1132:17
     |
1131 |         let (varname, subscripts) = self.get_buffers();
     |                                     ---- immutable borrow occurs here
1132 |         let t = self.subscripts.last_mut().unwrap_or(unsafe { self.variable.as_mut_vec() });
     |                 ^^^^^^^^^^^^^^^ mutable borrow occurs here
...
1145 |                     varname.as_ptr(),
     |                     ------- immutable borrow later used here

error[E0502]: cannot borrow `self.variable` as mutable because it is also borrowed as immutable
    --> src/simple_api/mod.rs:1132:63
     |
1131 |         let (varname, subscripts) = self.get_buffers();
     |                                     ---- immutable borrow occurs here
1132 |         let t = self.subscripts.last_mut().unwrap_or(unsafe { self.variable.as_mut_vec() });
     |                                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
...
1145 |                     varname.as_ptr(),
     |                     ------- immutable borrow later used here

error: aborting due to 2 previous errors

I have a nagging feeling there's unsoundness in sub_self_call, but I can't put my finger on it. It would be nice to fix this.

Edited by Jynn Nelson