From: "Onur Özkan" <work@onurozkan.dev>
To: rust-for-linux@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, daniel@sedlak.dev,
dirk.behme@de.bosch.com, felipe_life@live.com, tamird@gmail.com,
dakr@kernel.org, tmgross@umich.edu, aliceryhl@google.com,
a.hindborg@kernel.org, lossin@kernel.org,
bjorn3_gh@protonmail.com, gary@garyguo.net, boqun.feng@gmail.com,
alex.gaynor@gmail.com, ojeda@kernel.org,
"Onur Özkan" <work@onurozkan.dev>
Subject: [PATCH v1] rust: refactor `to_result` to return the original value
Date: Tue, 9 Sep 2025 15:32:58 +0300 [thread overview]
Message-ID: <20250909123258.29304-1-work@onurozkan.dev> (raw)
Current `to_result` helper takes a `c_int` and returns `Ok(())` on
success and this has some issues like:
- Callers lose the original return value and often have to store
it in a temporary variable before calling `to_result`.
- It only supports `c_int`, which makes callers to unnecessarily
cast when working with other types (e.g. `u16` in phy
abstractions). We even have some places that ignore to use
`to_result` helper because the input doesn't fit in `c_int`
(see [0]).
[0]: https://lore.kernel.org/all/20250822080252.773d6f54@nimda.home/
This patch changes `to_result` to be generic and also return the
original value on success.
So that the code that previously looked like:
let ret = unsafe { bindings::some_ffi_call() };
to_result(ret).map(|()| SomeType::new(ret))
can now be written more directly as:
to_result(unsafe { bindings::some_ffi_call() })
.map(|ret| SomeType::new(ret))
Similarly, code such as:
let res: isize = $some_ffi_call();
if res < 0 {
return Err(Error::from_errno(res as i32));
}
can now be used with `to_result` as:
to_result($some_ffi_call())?;
Existing call sites that only care about success/failure are updated
to append `.map(|_| ())` to preserve their previous semantics. They
can also use the equivalent pattern:
to_result($something)?;
Ok(())
This patch only fixes the callers that broke after the changes on `to_result`.
I haven't included all the improvements made possible by the new design since
that could conflict with other ongoing patches [1]. Once this patch is approved
and applied, I am planning to follow up with creating a "good first issue" on [2]
for those additional changes.
[1]: https://lore.kernel.org/rust-for-linux/?q=to_result
[2]: https://github.com/Rust-for-Linux/linux
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089/topic/x/near/536374456
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
rust/kernel/auxiliary.rs | 1 +
rust/kernel/block/mq/tag_set.rs | 2 +-
rust/kernel/cpufreq.rs | 3 ++-
rust/kernel/devres.rs | 1 +
rust/kernel/dma.rs | 3 +++
rust/kernel/error.rs | 17 ++++++++++++-----
rust/kernel/miscdevice.rs | 2 +-
rust/kernel/mm/virt.rs | 1 +
rust/kernel/pci.rs | 3 ++-
rust/kernel/platform.rs | 2 +-
rust/kernel/regulator.rs | 5 +++--
11 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 4749fb6bffef..479c0ad2a572 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -43,6 +43,7 @@ unsafe fn register(
to_result(unsafe {
bindings::__auxiliary_driver_register(adrv.get(), module.0, name.as_char_ptr())
})
+ .map(|_| ())
}
unsafe fn unregister(adrv: &Opaque<Self::RegType>) {
diff --git a/rust/kernel/block/mq/tag_set.rs b/rust/kernel/block/mq/tag_set.rs
index c3cf56d52bee..0e7883163000 100644
--- a/rust/kernel/block/mq/tag_set.rs
+++ b/rust/kernel/block/mq/tag_set.rs
@@ -65,7 +65,7 @@ pub fn new(
// SAFETY: we do not move out of `tag_set`.
let tag_set: &mut Opaque<_> = unsafe { Pin::get_unchecked_mut(tag_set) };
// SAFETY: `tag_set` is a reference to an initialized `blk_mq_tag_set`.
- error::to_result( unsafe { bindings::blk_mq_alloc_tag_set(tag_set.get())})
+ error::to_result( unsafe { bindings::blk_mq_alloc_tag_set(tag_set.get())}).map(|_| ())
}),
_p: PhantomData,
})
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index be2dffbdb546..c3fa20ce229a 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -157,6 +157,7 @@ pub fn as_raw(&self) -> *mut bindings::cpufreq_policy_data {
pub fn generic_verify(&self) -> Result {
// SAFETY: By the type invariant, the pointer stored in `self` is valid.
to_result(unsafe { bindings::cpufreq_generic_frequency_table_verify(self.as_raw()) })
+ .map(|_| ())
}
}
@@ -519,7 +520,7 @@ pub fn set_suspend_freq(&mut self, freq: Hertz) -> &mut Self {
#[inline]
pub fn generic_suspend(&mut self) -> Result {
// SAFETY: By the type invariant, the pointer stored in `self` is valid.
- to_result(unsafe { bindings::cpufreq_generic_suspend(self.as_mut_ref()) })
+ to_result(unsafe { bindings::cpufreq_generic_suspend(self.as_mut_ref()) }).map(|_| ())
}
/// Provides a wrapper to the generic get routine.
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index d04e3fcebafb..214cd9a0ebe0 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -328,6 +328,7 @@ fn register_foreign<P>(dev: &Device<Bound>, data: P) -> Result
// `ForeignOwnable` is released eventually.
bindings::devm_add_action_or_reset(dev.as_raw(), Some(callback::<P>), ptr.cast())
})
+ .map(|_| ())
}
/// Encapsulate `data` in a [`KBox`] and [`Drop::drop`] `data` once `dev` is unbound.
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 68fe67624424..f614453ddb7d 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -34,6 +34,7 @@ unsafe fn dma_set_mask(&self, mask: DmaMask) -> Result {
// - The safety requirement of this function guarantees that there are no concurrent calls
// to DMA allocation and mapping primitives using this mask.
to_result(unsafe { bindings::dma_set_mask(self.as_ref().as_raw(), mask.value()) })
+ .map(|_| ())
}
/// Set up the device's DMA coherent addressing capabilities.
@@ -51,6 +52,7 @@ unsafe fn dma_set_coherent_mask(&self, mask: DmaMask) -> Result {
// - The safety requirement of this function guarantees that there are no concurrent calls
// to DMA allocation and mapping primitives using this mask.
to_result(unsafe { bindings::dma_set_coherent_mask(self.as_ref().as_raw(), mask.value()) })
+ .map(|_| ())
}
/// Set up the device's DMA addressing capabilities.
@@ -72,6 +74,7 @@ unsafe fn dma_set_mask_and_coherent(&self, mask: DmaMask) -> Result {
to_result(unsafe {
bindings::dma_set_mask_and_coherent(self.as_ref().as_raw(), mask.value())
})
+ .map(|_| ())
}
}
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index db14da976722..f76afa4b7ec1 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -378,12 +378,19 @@ fn from(e: core::convert::Infallible) -> Error {
pub type Result<T = (), E = Error> = core::result::Result<T, E>;
/// Converts an integer as returned by a C kernel function to an error if it's negative, and
-/// `Ok(())` otherwise.
-pub fn to_result(err: crate::ffi::c_int) -> Result {
- if err < 0 {
- Err(Error::from_errno(err))
+/// returns the original value otherwise.
+pub fn to_result<T>(code: T) -> Result<T>
+where
+ T: Copy + TryInto<i32>,
+{
+ // Try casting into `i32`.
+ let casted: crate::ffi::c_int = code.try_into().unwrap_or(0);
+
+ if casted < 0 {
+ Err(Error::from_errno(casted))
} else {
- Ok(())
+ // Return the original input value.
+ Ok(code)
}
}
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 6373fe183b27..22b72ae84c03 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -79,7 +79,7 @@ pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
// the destructor of this type deallocates the memory.
// INVARIANT: If this returns `Ok(())`, then the `slot` will contain a registered
// misc device.
- to_result(unsafe { bindings::misc_register(slot) })
+ to_result(unsafe { bindings::misc_register(slot) }).map(|_| ())
}),
_t: PhantomData,
})
diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs
index a1bfa4e19293..5494f96e91b0 100644
--- a/rust/kernel/mm/virt.rs
+++ b/rust/kernel/mm/virt.rs
@@ -195,6 +195,7 @@ pub fn vm_insert_page(&self, address: usize, page: &Page) -> Result {
// SAFETY: By the type invariant of `Self` caller has read access and has verified that
// `VM_MIXEDMAP` is set. By invariant on `Page` the page has order 0.
to_result(unsafe { bindings::vm_insert_page(self.as_ptr(), address, page.as_ptr()) })
+ .map(|_| ())
}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 887ee611b553..6e917752cb89 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -48,6 +48,7 @@ unsafe fn register(
to_result(unsafe {
bindings::__pci_register_driver(pdrv.get(), module.0, name.as_char_ptr())
})
+ .map(|_| ())
}
unsafe fn unregister(pdrv: &Opaque<Self::RegType>) {
@@ -437,7 +438,7 @@ impl Device<device::Core> {
/// Enable memory resources for this device.
pub fn enable_device_mem(&self) -> Result {
// SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
- to_result(unsafe { bindings::pci_enable_device_mem(self.as_raw()) })
+ to_result(unsafe { bindings::pci_enable_device_mem(self.as_raw()) }).map(|_| ())
}
/// Enable bus-mastering for this device.
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 8f028c76f9fa..5a5561c7326e 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -54,7 +54,7 @@ unsafe fn register(
}
// SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
- to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), module.0) })
+ to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), module.0) }).map(|_| ())
}
unsafe fn unregister(pdrv: &Opaque<Self::RegType>) {
diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs
index 65f3a125348f..e17ae6e9a990 100644
--- a/rust/kernel/regulator.rs
+++ b/rust/kernel/regulator.rs
@@ -261,6 +261,7 @@ pub fn set_voltage(&self, min_voltage: Voltage, max_voltage: Voltage) -> Result
max_voltage.as_microvolts(),
)
})
+ .map(|_| ())
}
/// Gets the current voltage of the regulator.
@@ -291,12 +292,12 @@ fn get_internal(dev: &Device, name: &CStr) -> Result<Regulator<T>> {
fn enable_internal(&mut self) -> Result {
// SAFETY: Safe as per the type invariants of `Regulator`.
- to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) })
+ to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) }).map(|_| ())
}
fn disable_internal(&mut self) -> Result {
// SAFETY: Safe as per the type invariants of `Regulator`.
- to_result(unsafe { bindings::regulator_disable(self.inner.as_ptr()) })
+ to_result(unsafe { bindings::regulator_disable(self.inner.as_ptr()) }).map(|_| ())
}
}
--
2.50.0
next reply other threads:[~2025-09-09 12:39 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-09 12:32 Onur Özkan [this message]
2025-09-09 12:36 ` [PATCH v1] rust: refactor `to_result` to return the original value Alice Ryhl
2025-09-09 12:53 ` Daniel Almeida
2025-09-09 13:37 ` Daniel Almeida
2025-09-09 15:46 ` Onur
2025-09-09 15:45 ` Onur
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250909123258.29304-1-work@onurozkan.dev \
--to=work@onurozkan.dev \
--cc=a.hindborg@kernel.org \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=daniel@sedlak.dev \
--cc=dirk.behme@de.bosch.com \
--cc=felipe_life@live.com \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tamird@gmail.com \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox