From b5f23b00cd670f8817c97825ea200ec344b39414 Mon Sep 17 00:00:00 2001 From: beviu Date: Sat, 20 Sep 2025 22:15:01 +0200 Subject: [PATCH] Use bcache_fs_open_fallible in BcachefsHandle::open Also, as long as the path is valid, bcache_fs_open{,fallible} do not do anything unsafe so BcachefsHandle::open does not need to be marked unsafe. --- bch_bindgen/build.rs | 2 +- src/commands/subvolume.rs | 8 +++++--- src/wrappers/handle.rs | 20 ++++++++++---------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/bch_bindgen/build.rs b/bch_bindgen/build.rs index 10d5e575..c923f954 100644 --- a/bch_bindgen/build.rs +++ b/bch_bindgen/build.rs @@ -53,7 +53,7 @@ fn main() { .allowlist_function("cmd_.*") .allowlist_function(".*_cmds") .allowlist_function(".*bch2_.*") - .allowlist_function("bcache_fs_open") + .allowlist_function("bcache_fs_open_fallible") .allowlist_function("bcache_fs_close") .allowlist_function("bio_.*") .allowlist_function("derive_passphrase") diff --git a/src/commands/subvolume.rs b/src/commands/subvolume.rs index 6f5f2f2b..fb94ea8f 100644 --- a/src/commands/subvolume.rs +++ b/src/commands/subvolume.rs @@ -54,7 +54,8 @@ pub fn subvolume(argv: Vec) -> Result<()> { }; if let Some(dirname) = target.parent() { - let fs = unsafe { BcachefsHandle::open(dirname) }; + let fs = + BcachefsHandle::open(dirname).context("Failed to open the filesystem")?; fs.create_subvolume(target) .context("Failed to create the subvolume")?; } @@ -67,7 +68,8 @@ pub fn subvolume(argv: Vec) -> Result<()> { .context("subvolume path does not exist or can not be canonicalized")?; if let Some(dirname) = target.parent() { - let fs = unsafe { BcachefsHandle::open(dirname) }; + let fs = + BcachefsHandle::open(dirname).context("Failed to open the filesystem")?; fs.delete_subvolume(target) .context("Failed to delete the subvolume")?; } @@ -85,7 +87,7 @@ pub fn subvolume(argv: Vec) -> Result<()> { } else { dirname }; - let fs = unsafe { BcachefsHandle::open(dir) }; + let fs = BcachefsHandle::open(dir).context("Failed to open the filesystem")?; fs.snapshot_subvolume( if read_only { diff --git a/src/wrappers/handle.rs b/src/wrappers/handle.rs index a94f7305..f85637d0 100644 --- a/src/wrappers/handle.rs +++ b/src/wrappers/handle.rs @@ -1,13 +1,13 @@ +use std::mem::MaybeUninit; use std::path::Path; use bch_bindgen::c::{ - bcache_fs_close, bcache_fs_open, bch_ioctl_subvolume, bch_ioctl_subvolume_v2, bch_ioctl_err_msg, bchfs_handle, - BCH_IOCTL_SUBVOLUME_CREATE, - BCH_IOCTL_SUBVOLUME_CREATE_v2, - BCH_IOCTL_SUBVOLUME_DESTROY, - BCH_IOCTL_SUBVOLUME_DESTROY_v2, + bcache_fs_close, bcache_fs_open_fallible, bch_errcode, bch_ioctl_err_msg, bch_ioctl_subvolume, + bch_ioctl_subvolume_v2, bchfs_handle, BCH_IOCTL_SUBVOLUME_CREATE_v2, + BCH_IOCTL_SUBVOLUME_DESTROY_v2, BCH_IOCTL_SUBVOLUME_CREATE, BCH_IOCTL_SUBVOLUME_DESTROY, BCH_SUBVOL_SNAPSHOT_CREATE, }; +use bch_bindgen::errcode::ret_to_result; use bch_bindgen::path_to_cstr; use errno::Errno; @@ -19,12 +19,12 @@ pub(crate) struct BcachefsHandle { impl BcachefsHandle { /// Opens a bcachefs filesystem and returns its handle - /// TODO(raitobezarius): how can this not be faillible? - pub(crate) unsafe fn open>(path: P) -> Self { + pub(crate) fn open>(path: P) -> Result { let path = path_to_cstr(path); - Self { - inner: bcache_fs_open(path.as_ptr()), - } + let mut handle: MaybeUninit = MaybeUninit::uninit(); + ret_to_result(unsafe { bcache_fs_open_fallible(path.as_ptr(), handle.as_mut_ptr()) })?; + let inner = unsafe { handle.assume_init() }; + Ok(Self { inner }) } }