Skip to content

Commit bf173b5

Browse files
authored
Merge pull request #77 from orbitinghail/safer-handle
Use ptr::write/ptr::read to initialize and destroy file handle
2 parents 7d1d078 + 92beaea commit bf173b5

1 file changed

Lines changed: 49 additions & 21 deletions

File tree

‎src/vfs.rs‎

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use alloc::boxed::Box;
77
use alloc::ffi::CString;
88
use alloc::format;
99
use alloc::string::String;
10-
use core::mem::{self, ManuallyDrop, MaybeUninit, size_of};
10+
use core::mem::{ManuallyDrop, size_of};
1111
use core::slice;
1212
use core::{
1313
ffi::{CStr, c_char, c_int, c_void},
@@ -45,7 +45,7 @@ pub type VfsResult<T> = Result<T, SqliteErr>;
4545
struct FileWrapper<Handle> {
4646
file: ffi::sqlite3_file,
4747
vfs: *mut ffi::sqlite3_vfs,
48-
handle: MaybeUninit<Handle>,
48+
handle: Handle,
4949
}
5050

5151
struct AppData<Vfs> {
@@ -371,7 +371,6 @@ unsafe extern "C" fn x_open<T: Vfs>(
371371
let vfs = unwrap_vfs!(p_vfs, T)?;
372372
let handle = vfs.open(name.as_ref().map(|s| s.as_ref()), opts)?;
373373

374-
let out_file = unwrap_file!(p_file, T)?;
375374
let appdata = unwrap_appdata!(p_vfs, T)?;
376375

377376
if let Some(p_out_flags) = unsafe { p_out_flags.as_mut() } {
@@ -385,9 +384,19 @@ unsafe extern "C" fn x_open<T: Vfs>(
385384
*p_out_flags = out_flags;
386385
}
387386

388-
out_file.file.pMethods = &appdata.io_methods;
389-
out_file.vfs = p_vfs;
390-
out_file.handle.write(handle);
387+
let out_file = p_file.cast::<FileWrapper<T::Handle>>();
388+
// Safety: SQLite owns the heap allocation backing p_file (it handles malloc/free).
389+
// We use ptr::write to initialize that allocation directly.
390+
unsafe {
391+
core::ptr::write(
392+
out_file,
393+
FileWrapper {
394+
file: ffi::sqlite3_file { pMethods: &appdata.io_methods },
395+
vfs: p_vfs,
396+
handle,
397+
},
398+
);
399+
}
391400

392401
Ok(vars::SQLITE_OK)
393402
})
@@ -447,10 +456,23 @@ unsafe extern "C" fn x_full_pathname<T: Vfs>(
447456

448457
unsafe extern "C" fn x_close<T: Vfs>(p_file: *mut ffi::sqlite3_file) -> c_int {
449458
fallible(|| {
450-
let file = unwrap_file!(p_file, T)?;
451-
let vfs = unwrap_vfs!(file.vfs, T)?;
452-
let handle = mem::replace(&mut file.handle, MaybeUninit::uninit());
453-
let handle = unsafe { handle.assume_init() };
459+
// Safety: SQLite owns the heap allocation backing p_file (it handles
460+
// malloc/free). We use ptr::read to copy our Handle out of that
461+
// allocation so it can be passed to vfs.close() and properly dropped.
462+
// SQLite will not call any other file methods after x_close without
463+
// first calling x_open to reinitialize the handle.
464+
let (vfs, handle) = unsafe {
465+
// verify p_file is not null and get a mutable reference
466+
let p_file_ref = p_file.as_mut().ok_or(vars::SQLITE_INTERNAL)?;
467+
// set pMethods to null, signaling to SQLite that the file is closed
468+
p_file_ref.pMethods = core::ptr::null();
469+
470+
// extract a copy of the FileWrapper
471+
let file = core::ptr::read(p_file.cast::<FileWrapper<T::Handle>>());
472+
(file.vfs, file.handle)
473+
};
474+
475+
let vfs = unwrap_vfs!(vfs, T)?;
454476
vfs.close(handle)?;
455477
Ok(vars::SQLITE_OK)
456478
})
@@ -468,7 +490,7 @@ unsafe extern "C" fn x_read<T: Vfs>(
468490
let buf_len: usize = i_amt.try_into().map_err(|_| vars::SQLITE_IOERR_READ)?;
469491
let offset: usize = i_ofst.try_into().map_err(|_| vars::SQLITE_IOERR_READ)?;
470492
let buf = unsafe { slice::from_raw_parts_mut(buf.cast::<u8>(), buf_len) };
471-
vfs.read(unsafe { file.handle.assume_init_mut() }, offset, buf)?;
493+
vfs.read(&mut file.handle, offset, buf)?;
472494
Ok(vars::SQLITE_OK)
473495
})
474496
}
@@ -485,7 +507,7 @@ unsafe extern "C" fn x_write<T: Vfs>(
485507
let buf_len: usize = i_amt.try_into().map_err(|_| vars::SQLITE_IOERR_WRITE)?;
486508
let offset: usize = i_ofst.try_into().map_err(|_| vars::SQLITE_IOERR_WRITE)?;
487509
let buf = unsafe { slice::from_raw_parts(buf.cast::<u8>(), buf_len) };
488-
let n = vfs.write(unsafe { file.handle.assume_init_mut() }, offset, buf)?;
510+
let n = vfs.write(&mut file.handle, offset, buf)?;
489511
if n != buf_len {
490512
return Err(vars::SQLITE_IOERR_WRITE);
491513
}
@@ -501,7 +523,7 @@ unsafe extern "C" fn x_truncate<T: Vfs>(
501523
let file = unwrap_file!(p_file, T)?;
502524
let vfs = unwrap_vfs!(file.vfs, T)?;
503525
let size: usize = size.try_into().map_err(|_| vars::SQLITE_IOERR_TRUNCATE)?;
504-
vfs.truncate(unsafe { file.handle.assume_init_mut() }, size)?;
526+
vfs.truncate(&mut file.handle, size)?;
505527
Ok(vars::SQLITE_OK)
506528
})
507529
}
@@ -510,7 +532,7 @@ unsafe extern "C" fn x_sync<T: Vfs>(p_file: *mut ffi::sqlite3_file, _flags: c_in
510532
fallible(|| {
511533
let file = unwrap_file!(p_file, T)?;
512534
let vfs = unwrap_vfs!(file.vfs, T)?;
513-
vfs.sync(unsafe { file.handle.assume_init_mut() })?;
535+
vfs.sync(&mut file.handle)?;
514536
Ok(vars::SQLITE_OK)
515537
})
516538
}
@@ -522,7 +544,7 @@ unsafe extern "C" fn x_file_size<T: Vfs>(
522544
fallible(|| {
523545
let file = unwrap_file!(p_file, T)?;
524546
let vfs = unwrap_vfs!(file.vfs, T)?;
525-
let size = vfs.file_size(unsafe { file.handle.assume_init_mut() })?;
547+
let size = vfs.file_size(&mut file.handle)?;
526548
let p_size = unsafe { p_size.as_mut() }.ok_or(vars::SQLITE_INTERNAL)?;
527549
*p_size = size.try_into().map_err(|_| vars::SQLITE_IOERR_FSTAT)?;
528550
Ok(vars::SQLITE_OK)
@@ -534,7 +556,7 @@ unsafe extern "C" fn x_lock<T: Vfs>(p_file: *mut ffi::sqlite3_file, raw_lock: c_
534556
let level: LockLevel = raw_lock.into();
535557
let file = unwrap_file!(p_file, T)?;
536558
let vfs = unwrap_vfs!(file.vfs, T)?;
537-
vfs.lock(unsafe { file.handle.assume_init_mut() }, level)?;
559+
vfs.lock(&mut file.handle, level)?;
538560
Ok(vars::SQLITE_OK)
539561
})
540562
}
@@ -544,7 +566,7 @@ unsafe extern "C" fn x_unlock<T: Vfs>(p_file: *mut ffi::sqlite3_file, raw_lock:
544566
let level: LockLevel = raw_lock.into();
545567
let file = unwrap_file!(p_file, T)?;
546568
let vfs = unwrap_vfs!(file.vfs, T)?;
547-
vfs.unlock(unsafe { file.handle.assume_init_mut() }, level)?;
569+
vfs.unlock(&mut file.handle, level)?;
548570
Ok(vars::SQLITE_OK)
549571
})
550572
}
@@ -557,7 +579,7 @@ unsafe extern "C" fn x_check_reserved_lock<T: Vfs>(
557579
let file = unwrap_file!(p_file, T)?;
558580
let vfs = unwrap_vfs!(file.vfs, T)?;
559581
unsafe {
560-
*p_out = vfs.check_reserved_lock(file.handle.assume_init_mut())? as c_int;
582+
*p_out = vfs.check_reserved_lock(&mut file.handle)? as c_int;
561583
}
562584
Ok(vars::SQLITE_OK)
563585
})
@@ -598,7 +620,7 @@ unsafe extern "C" fn x_file_control<T: Vfs>(
598620
};
599621
let pragma = Pragma { name: &name, arg: arg.as_deref() };
600622

601-
let (result, msg) = match vfs.pragma(unsafe { file.handle.assume_init_mut() }, pragma) {
623+
let (result, msg) = match vfs.pragma(&mut file.handle, pragma) {
602624
Ok(msg) => (Ok(vars::SQLITE_OK), msg),
603625
Err(PragmaErr::NotFound) => (Err(vars::SQLITE_NOTFOUND), None),
604626
Err(PragmaErr::Fail(err, msg)) => (Err(err), msg),
@@ -623,15 +645,15 @@ unsafe extern "C" fn x_sector_size<T: Vfs>(p_file: *mut ffi::sqlite3_file) -> c_
623645
fallible(|| {
624646
let file = unwrap_file!(p_file, T)?;
625647
let vfs = unwrap_vfs!(file.vfs, T)?;
626-
vfs.sector_size(unsafe { file.handle.assume_init_mut() })
648+
vfs.sector_size(&mut file.handle)
627649
})
628650
}
629651

630652
unsafe extern "C" fn x_device_characteristics<T: Vfs>(p_file: *mut ffi::sqlite3_file) -> c_int {
631653
fallible(|| {
632654
let file = unwrap_file!(p_file, T)?;
633655
let vfs = unwrap_vfs!(file.vfs, T)?;
634-
vfs.device_characteristics(unsafe { file.handle.assume_init_mut() })
656+
vfs.device_characteristics(&mut file.handle)
635657
})
636658
}
637659

@@ -818,13 +840,19 @@ mod tests {
818840
let n = blob.write(b"hello")?;
819841
assert_eq!(n, 5);
820842

843+
blob.close()?;
844+
821845
// query the table for the blob and print it
822846
let mut stmt = conn.prepare("select data from b")?;
823847
let mut rows = stmt.query([])?;
824848
while let Some(row) = rows.next()? {
825849
let data: Vec<u8> = row.get(0)?;
826850
assert_eq!(&data[0..5], b"hello");
827851
}
852+
drop(rows);
853+
drop(stmt);
854+
855+
conn.close().expect("failed to close connection");
828856

829857
Ok(())
830858
}

0 commit comments

Comments
 (0)