diff mbox series

[v6,2/4] rust: replace `CStr` with `core::ffi::CStr`

Message ID 20250202-cstr-core-v6-2-8469cd6d29fd@gmail.com
State Superseded
Headers show
Series rust: replace kernel::str::CStr w/ core::ffi::CStr | expand

Commit Message

Tamir Duberstein Feb. 2, 2025, 12:20 p.m. UTC
`std::ffi::CStr` was moved to `core::ffi::CStr` in Rust 1.64. Replace
`kernel::str::CStr` with `core::ffi::CStr` now that we can.

`kernel::str::CStr::{from,as}_char_ptr` live on as free functions in
`kernel::str` to paper over the difference between `core::ffi::c_char`
and `kernel::ffi::char`; see the code comment on the latter type for
details.

C-String literals were added in Rust 1.77. Opportunistically replace
instances of `kernel::c_str!` with C-String literals where other code
changes were already necessary; the rest will be done in a later commit.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 drivers/gpu/drm/drm_panic_qr.rs |   2 +-
 rust/kernel/device.rs           |   5 +-
 rust/kernel/error.rs            |   4 +-
 rust/kernel/firmware.rs         |   2 +-
 rust/kernel/kunit.rs            |   4 +-
 rust/kernel/miscdevice.rs       |   2 +-
 rust/kernel/net/phy.rs          |   2 +-
 rust/kernel/prelude.rs          |   5 +-
 rust/kernel/seq_file.rs         |   4 +-
 rust/kernel/str.rs              | 449 ++++++----------------------------------
 rust/kernel/sync/condvar.rs     |   2 +-
 rust/kernel/sync/lock.rs        |   2 +-
 rust/kernel/sync/lock/global.rs |   2 +-
 rust/kernel/workqueue.rs        |   2 +-
 14 files changed, 88 insertions(+), 399 deletions(-)

Comments

Tamir Duberstein Feb. 2, 2025, 12:54 p.m. UTC | #1
On Sun, Feb 2, 2025 at 7:20 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
>  impl DerefMut for CString {
>      fn deref_mut(&mut self) -> &mut Self::Target {
> -        // SAFETY: A `CString` is always NUL-terminated and contains no other
> -        // NUL bytes.
> -        unsafe { CStr::from_bytes_with_nul_unchecked_mut(self.buf.as_mut_slice()) }
> +        // SAFETY: Properties of `bytes` guaranteed by the safety precondition.
> +        unsafe { &mut *(self.buf.as_mut_slice() as *mut [u8] as *mut CStr) }
>      }
>  }

Whoops, this safety comment is nonsense. I'll use the one from `impl
Deref` above in v7...or just remove this impl altogether. I'm not sure
what the use for `&mut CStr` would be.
Tamir Duberstein Feb. 2, 2025, 2:40 p.m. UTC | #2
On Sun, Feb 2, 2025 at 7:54 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Sun, Feb 2, 2025 at 7:20 AM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> >  impl DerefMut for CString {
> >      fn deref_mut(&mut self) -> &mut Self::Target {
> > -        // SAFETY: A `CString` is always NUL-terminated and contains no other
> > -        // NUL bytes.
> > -        unsafe { CStr::from_bytes_with_nul_unchecked_mut(self.buf.as_mut_slice()) }
> > +        // SAFETY: Properties of `bytes` guaranteed by the safety precondition.
> > +        unsafe { &mut *(self.buf.as_mut_slice() as *mut [u8] as *mut CStr) }
> >      }
> >  }
>
> Whoops, this safety comment is nonsense. I'll use the one from `impl
> Deref` above in v7...or just remove this impl altogether. I'm not sure
> what the use for `&mut CStr` would be.

Ah, turns out Michal's patches removed functionality added in commit
a321f3ad0a5d ("rust: str: add {make,to}_{upper,lower}case() to
CString") which relied on this DerefMut impl. I've restored it all in
v7.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs
index bcf248f69252..ef294017888e 100644
--- a/drivers/gpu/drm/drm_panic_qr.rs
+++ b/drivers/gpu/drm/drm_panic_qr.rs
@@ -958,7 +958,7 @@  fn draw_all(&mut self, data: impl Iterator<Item = u8>) {
     } else {
         // SAFETY: The caller ensures that `url` is a valid pointer to a
         // nul-terminated string.
-        let url_cstr: &CStr = unsafe { CStr::from_char_ptr(url) };
+        let url_cstr: &CStr = unsafe { kernel::str::from_char_ptr(url) };
         let segments = &[
             &Segment::Binary(url_cstr.as_bytes()),
             &Segment::Numeric(&data_slice[0..data_len]),
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index d5e6a19ff6b7..f97c409aaba9 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -10,9 +10,6 @@ 
 };
 use core::{fmt, ptr};
 
-#[cfg(CONFIG_PRINTK)]
-use crate::c_str;
-
 /// A reference-counted device.
 ///
 /// This structure represents the Rust abstraction for a C `struct device`. This implementation
@@ -175,7 +172,7 @@  unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
             bindings::_dev_printk(
                 klevel as *const _ as *const crate::ffi::c_char,
                 self.as_raw(),
-                c_str!("%pA").as_char_ptr(),
+                crate::str::as_char_ptr(c"%pA"),
                 &msg as *const _ as *const crate::ffi::c_void,
             )
         };
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index f6ecf09cb65f..51c5e0f0a0bc 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -164,7 +164,7 @@  pub fn name(&self) -> Option<&'static CStr> {
             None
         } else {
             // SAFETY: The string returned by `errname` is static and `NUL`-terminated.
-            Some(unsafe { CStr::from_char_ptr(ptr) })
+            Some(unsafe { crate::str::from_char_ptr(ptr) })
         }
     }
 
@@ -187,7 +187,7 @@  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
             Some(name) => f
                 .debug_tuple(
                     // SAFETY: These strings are ASCII-only.
-                    unsafe { core::str::from_utf8_unchecked(name) },
+                    unsafe { core::str::from_utf8_unchecked(name.to_bytes()) },
                 )
                 .finish(),
         }
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index c5162fdc95ff..e75db4d825ce 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -61,7 +61,7 @@  fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result<Self> {
 
         // SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
         // `name` and `dev` are valid as by their type invariants.
-        let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) };
+        let ret = unsafe { func.0(pfw as _, crate::str::as_char_ptr(name), dev.as_raw()) };
         if ret != 0 {
             return Err(Error::from_errno(ret));
         }
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 630b947c708c..9f40ea744fc2 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -101,12 +101,12 @@  unsafe impl Sync for Location {}
             unsafe impl Sync for UnaryAssert {}
 
             static LOCATION: Location = Location($crate::bindings::kunit_loc {
-                file: FILE.as_char_ptr(),
+                file: $crate::str::as_char_ptr(FILE),
                 line: LINE,
             });
             static ASSERTION: UnaryAssert = UnaryAssert($crate::bindings::kunit_unary_assert {
                 assert: $crate::bindings::kunit_assert {},
-                condition: CONDITION.as_char_ptr(),
+                condition: $crate::str::as_char_ptr(CONDITION),
                 expected_true: true,
             });
 
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index b3a6cc50b240..2dd20e981e9b 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -31,7 +31,7 @@  pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
         // SAFETY: All zeros is valid for this C type.
         let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
         result.minor = bindings::MISC_DYNAMIC_MINOR as _;
-        result.name = self.name.as_char_ptr();
+        result.name = crate::str::as_char_ptr(self.name);
         result.fops = create_vtable::<T>();
         result
     }
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index d7da29f95e43..9e410de3c3a3 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -504,7 +504,7 @@  unsafe impl Sync for DriverVTable {}
 pub const fn create_phy_driver<T: Driver>() -> DriverVTable {
     // INVARIANT: All the fields of `struct phy_driver` are initialized properly.
     DriverVTable(Opaque::new(bindings::phy_driver {
-        name: T::NAME.as_char_ptr().cast_mut(),
+        name: crate::str::as_char_ptr(T::NAME),
         flags: T::FLAGS,
         phy_id: T::PHY_DEVICE_ID.id,
         phy_id_mask: T::PHY_DEVICE_ID.mask_as_int(),
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index dde2e0649790..96e7029c27da 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -34,7 +34,10 @@ 
 
 pub use super::error::{code::*, Error, Result};
 
-pub use super::{str::CStr, ThisModule};
+pub use super::{
+    str::{CStr, CStrExt as _},
+    ThisModule,
+};
 
 pub use super::init::{InPlaceInit, InPlaceWrite, Init, PinInit};
 
diff --git a/rust/kernel/seq_file.rs b/rust/kernel/seq_file.rs
index 04947c672979..bb3c49fd4ef3 100644
--- a/rust/kernel/seq_file.rs
+++ b/rust/kernel/seq_file.rs
@@ -4,7 +4,7 @@ 
 //!
 //! C header: [`include/linux/seq_file.h`](srctree/include/linux/seq_file.h)
 
-use crate::{bindings, c_str, types::NotThreadSafe, types::Opaque};
+use crate::{bindings, types::NotThreadSafe, types::Opaque};
 
 /// A utility for generating the contents of a seq file.
 #[repr(transparent)]
@@ -35,7 +35,7 @@  pub fn call_printf(&self, args: core::fmt::Arguments<'_>) {
         unsafe {
             bindings::seq_printf(
                 self.inner.get(),
-                c_str!("%pA").as_char_ptr(),
+                crate::str::as_char_ptr(c"%pA"),
                 &args as *const _ as *const crate::ffi::c_void,
             );
         }
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 53450c956a6a..2b63cfaaa981 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -4,7 +4,7 @@ 
 
 use crate::alloc::{flags::*, AllocError, KVec};
 use core::fmt::{self, Write};
-use core::ops::{self, Deref, DerefMut, Index};
+use core::ops::{Deref, DerefMut};
 
 use crate::error::{code::*, Error};
 
@@ -45,11 +45,11 @@  pub const fn from_bytes(bytes: &[u8]) -> &Self {
     /// # use kernel::{fmt, b_str, str::CString};
     /// let ascii = b_str!("Hello, BStr!");
     /// let s = CString::try_from_fmt(fmt!("{}", ascii.display()))?;
-    /// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes());
+    /// assert_eq!(s.to_bytes(), "Hello, BStr!".as_bytes());
     ///
     /// let non_ascii = b_str!("