| Message ID | 20250413-rust_arm_fix_fw_abstaction-v3-1-8dd7c0bbcd47@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] rust: Use `ffi::c_char` type in firmware abstraction `FwFunc` | expand |
On Sun, Apr 13, 2025 at 9:27 PM Christian Schrefl <chrisi.schrefl@gmail.com> wrote: > > The `FwFunc` struct contains an function with a char pointer argument, > for which a `*const u8` pointer was used. This is not really the > "proper" type for this, so use a `*const kernel::ffi::c_char` pointer > instead. If I don't take it: Acked-by: Miguel Ojeda <ojeda@kernel.org> Thanks! Cheers, Miguel
On Sun, Apr 13, 2025 at 09:26:56PM +0200, Christian Schrefl wrote: > The `FwFunc` struct contains an function with a char pointer argument, > for which a `*const u8` pointer was used. This is not really the > "proper" type for this, so use a `*const kernel::ffi::c_char` pointer > instead. With the following changes, applied to driver-core-linus, thanks! * add firmware prefix to commit subject - Danilo
On Sun, Apr 13, 2025 at 10:21:01PM +0200, Miguel Ojeda wrote: > On Sun, Apr 13, 2025 at 9:27 PM Christian Schrefl > <chrisi.schrefl@gmail.com> wrote: > > > > The `FwFunc` struct contains an function with a char pointer argument, > > for which a `*const u8` pointer was used. This is not really the > > "proper" type for this, so use a `*const kernel::ffi::c_char` pointer > > instead. > > If I don't take it: > > Acked-by: Miguel Ojeda <ojeda@kernel.org> Thanks, I'll take it now. greg k-h
On Tue, Apr 15, 2025 at 04:57:45PM +0200, Greg Kroah-Hartman wrote: > On Sun, Apr 13, 2025 at 10:21:01PM +0200, Miguel Ojeda wrote: > > On Sun, Apr 13, 2025 at 9:27 PM Christian Schrefl > > <chrisi.schrefl@gmail.com> wrote: > > > > > > The `FwFunc` struct contains an function with a char pointer argument, > > > for which a `*const u8` pointer was used. This is not really the > > > "proper" type for this, so use a `*const kernel::ffi::c_char` pointer > > > instead. > > > > If I don't take it: > > > > Acked-by: Miguel Ojeda <ojeda@kernel.org> > > Thanks, I'll take it now. Oops, it's already in the driver-core tree :)
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs index f04b058b09b2d2397e26344d0e055b3aa5061432..2494c96e105f3a28af74548d63a44464ba50eae3 100644 --- a/rust/kernel/firmware.rs +++ b/rust/kernel/firmware.rs @@ -4,7 +4,7 @@ //! //! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h) -use crate::{bindings, device::Device, error::Error, error::Result, str::CStr}; +use crate::{bindings, device::Device, error::Error, error::Result, ffi, str::CStr}; use core::ptr::NonNull; /// # Invariants @@ -12,7 +12,11 @@ /// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`, /// `bindings::firmware_request_platform`, `bindings::request_firmware_direct`. struct FwFunc( - unsafe extern "C" fn(*mut *const bindings::firmware, *const u8, *mut bindings::device) -> i32, + unsafe extern "C" fn( + *mut *const bindings::firmware, + *const ffi::c_char, + *mut bindings::device, + ) -> i32, ); impl FwFunc {
The `FwFunc` struct contains an function with a char pointer argument, for which a `*const u8` pointer was used. This is not really the "proper" type for this, so use a `*const kernel::ffi::c_char` pointer instead. This has no real functionality changes, since now `kernel::ffi::c_char` (which bindgen uses for `char`) is now a type alias to `u8` anyways, but before commit 1bae8729e50a ("rust: map `long` to `isize` and `char` to `u8`") the concrete type of `kernel::ffi::c_char` depended on the architecture (However all supported architectures at the time mapped to `i8`). This caused problems on the v6.13 tag when building for 32 bit arm (with my patches), since back then `*const i8` was used in the function argument and the function that bindgen generated used `*const core::ffi::c_char` which Rust mapped to `*const u8` on 32 bit arm. The stable v6.13.y branch does not have this issue since commit 1bae8729e50a ("rust: map `long` to `isize` and `char` to `u8`") was backported. This caused the following build error: ``` error[E0308]: mismatched types --> rust/kernel/firmware.rs:20:4 | 20 | Self(bindings::request_firmware) | ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item | | | arguments to this function are incorrect | = note: expected fn pointer `unsafe extern "C" fn(_, *const i8, _) -> _` found fn item `unsafe extern "C" fn(_, *const u8, _) -> _ {request_firmware}` note: tuple struct defined here --> rust/kernel/firmware.rs:14:8 | 14 | struct FwFunc( | ^^^^^^ error[E0308]: mismatched types --> rust/kernel/firmware.rs:24:14 | 24 | Self(bindings::firmware_request_nowarn) | ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item | | | arguments to this function are incorrect | = note: expected fn pointer `unsafe extern "C" fn(_, *const i8, _) -> _` found fn item `unsafe extern "C" fn(_, *const u8, _) -> _ {firmware_request_nowarn}` note: tuple struct defined here --> rust/kernel/firmware.rs:14:8 | 14 | struct FwFunc( | ^^^^^^ error[E0308]: mismatched types --> rust/kernel/firmware.rs:64:45 | 64 | let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) }; | ------ ^^^^^^^^^^^^^^^^^^ expected `*const i8`, found `*const u8` | | | arguments to this function are incorrect | = note: expected raw pointer `*const i8` found raw pointer `*const u8` error: aborting due to 3 previous errors ``` Fixes: de6582833db0 ("rust: add firmware abstractions") Cc: stable@vger.kernel.org Reviewed-by: Benno Lossin <benno.lossin@proton.me> Acked-by: Danilo Krummrich <dakr@kernel.org> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com> --- Changes in v3: - Clarify build issues with v6.13 in commit message. - Link to v2: https://lore.kernel.org/r/20250412-rust_arm_fix_fw_abstaction-v2-1-8e6fdf093d71@gmail.com Changes in v2: - Use `kernel::ffi::c_char` instead of `core::ffi::c_char`. (Danilo & Benno) - Reword the commit message. - Link to v1: https://lore.kernel.org/r/20250411-rust_arm_fix_fw_abstaction-v1-1-0a9e598451c6@gmail.com --- rust/kernel/firmware.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) --- base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8 change-id: 20250408-rust_arm_fix_fw_abstaction-4c3a89d75e29 Best regards,