* [PATCH] rust: rename `kernel::c_str!` to `str_to_cstr!`
@ 2026-03-02 17:20 Tamir Duberstein
2026-03-02 17:31 ` Gary Guo
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Tamir Duberstein @ 2026-03-02 17:20 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Breno Leitao, David Airlie, Simona Vetter,
Brendan Higgins, David Gow, Rae Moar, Peter Zijlstra, Ingo Molnar,
Will Deacon, Waiman Long
Cc: rust-for-linux, linux-kernel, dri-devel, linux-kselftest,
kunit-dev, Greg Kroah-Hartman, Tamir Duberstein
Now that all literals are C-Strings, rename and update the documentation
of this macro to clarify its intended purpose.
Link: https://github.com/Rust-for-Linux/linux/issues/1075
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Tamir Duberstein <tamird@kernel.org>
---
This patch completes the work of replacing our custom `CStr` with
upstream's.
---
rust/kernel/bug.rs | 2 +-
rust/kernel/configfs.rs | 2 +-
rust/kernel/drm/ioctl.rs | 2 +-
rust/kernel/kunit.rs | 3 ++-
rust/kernel/str.rs | 19 ++++++++++++++-----
rust/kernel/sync.rs | 4 ++--
rust/kernel/sync/lock/global.rs | 3 ++-
rust/kernel/workqueue.rs | 6 +++---
8 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs
index ed943960f851..f7cb673b1766 100644
--- a/rust/kernel/bug.rs
+++ b/rust/kernel/bug.rs
@@ -80,7 +80,7 @@ macro_rules! warn_flags {
// with a valid null-terminated string.
unsafe {
$crate::bindings::warn_slowpath_fmt(
- $crate::c_str!(::core::file!()).as_char_ptr(),
+ $crate::str_to_cstr!(::core::file!()).as_char_ptr(),
line!() as $crate::ffi::c_int,
$flags as $crate::ffi::c_uint,
::core::ptr::null(),
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 2339c6467325..930f17bb2041 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -1000,7 +1000,7 @@ macro_rules! configfs_attrs {
$crate::configfs::Attribute<$attr, $data, $data> =
unsafe {
$crate::configfs::Attribute::new(
- $crate::c_str!(::core::stringify!($name)),
+ $crate::str_to_cstr!(::core::stringify!($name)),
)
};
)*
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index cf328101dde4..6a87f489dc88 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -157,7 +157,7 @@ macro_rules! declare_drm_ioctls {
},
flags: $flags,
name: $crate::str::as_char_ptr_in_const_context(
- $crate::c_str!(::core::stringify!($cmd)),
+ $crate::str_to_cstr!(::core::stringify!($cmd)),
),
}
),*];
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index f93f24a60bdd..5802a3507ecc 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -59,7 +59,8 @@ macro_rules! kunit_assert {
static FILE: &'static $crate::str::CStr = $file;
static LINE: i32 = ::core::line!() as i32 - $diff;
- static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
+ static CONDITION: &'static $crate::str::CStr =
+ $crate::str_to_cstr!(stringify!($condition));
// SAFETY: FFI call without safety requirements.
let kunit_test = unsafe { $crate::bindings::kunit_get_current_test() };
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index fa87779d2253..8bb40de007d4 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -376,19 +376,28 @@ fn as_ref(&self) -> &BStr {
}
}
-/// Creates a new [`CStr`] from a string literal.
+/// Creates a new [`CStr`] at compile time.
///
-/// The string literal should not contain any `NUL` bytes.
+/// Rust supports C string literals since Rust 1.77, and they should be used instead of this macro
+/// where possible. This macro exists to allow static *non-literal* C strings to be created at
+/// compile time. This is most often used in other macros.
+///
+/// # Panics
+///
+/// This macro panics if the operand contains an interior `NUL` byte.
///
/// # Examples
///
/// ```
-/// # use kernel::c_str;
+/// # use kernel::str_to_cstr;
/// # use kernel::str::CStr;
-/// const MY_CSTR: &CStr = c_str!("My awesome CStr!");
+/// const MY_CSTR: &CStr = str_to_cstr!(concat!(file!(), ":", line!(), ": My CStr!"));
/// ```
#[macro_export]
-macro_rules! c_str {
+macro_rules! str_to_cstr {
+ // NB: we could write `($str:lit) => compile_error!("use a C string literal instead");` here but
+ // that would trigger when the literal is at the top of several macro expansions. That would be
+ // too limiting to macro authors, so we rely on the name as a hint instead.
($str:expr) => {{
const S: &str = concat!($str, "\0");
const C: &$crate::str::CStr = match $crate::str::CStr::from_bytes_with_nul(S.as_bytes()) {
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 993dbf2caa0e..ecf02a67ec35 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -154,9 +154,9 @@ macro_rules! static_lock_class {
#[macro_export]
macro_rules! optional_name {
() => {
- $crate::c_str!(::core::concat!(::core::file!(), ":", ::core::line!()))
+ $crate::str_to_cstr!(::core::concat!(::core::file!(), ":", ::core::line!()))
};
($name:literal) => {
- $crate::c_str!($name)
+ $crate::str_to_cstr!($name)
};
}
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index aecbdc34738f..81f46229be7d 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -272,7 +272,8 @@ macro_rules! global_lock {
$pub enum $name {}
impl $crate::sync::lock::GlobalLockBackend for $name {
- const NAME: &'static $crate::str::CStr = $crate::c_str!(::core::stringify!($name));
+ const NAME: &'static $crate::str::CStr =
+ $crate::str_to_cstr!(::core::stringify!($name));
type Item = $valuety;
type Backend = $crate::global_lock_inner!(backend $kind);
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 706e833e9702..7616d71df68e 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -212,7 +212,7 @@ macro_rules! new_delayed_work {
$crate::workqueue::DelayedWork::new(
$crate::optional_name!(),
$crate::static_lock_class!(),
- $crate::c_str!(::core::concat!(
+ $crate::str_to_cstr!(::core::concat!(
::core::file!(),
":",
::core::line!(),
@@ -223,9 +223,9 @@ macro_rules! new_delayed_work {
};
($name:literal) => {
$crate::workqueue::DelayedWork::new(
- $crate::c_str!($name),
+ $crate::str_to_cstr!($name),
$crate::static_lock_class!(),
- $crate::c_str!(::core::concat!($name, "_timer")),
+ $crate::str_to_cstr!(::core::concat!($name, "_timer")),
$crate::static_lock_class!(),
)
};
---
base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
change-id: 20260302-cstr-rename-macro-64201be6c969
Best regards,
--
Tamir Duberstein <tamird@kernel.org>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] rust: rename `kernel::c_str!` to `str_to_cstr!`
2026-03-02 17:20 [PATCH] rust: rename `kernel::c_str!` to `str_to_cstr!` Tamir Duberstein
@ 2026-03-02 17:31 ` Gary Guo
2026-03-02 17:45 ` Tamir Duberstein
2026-03-03 3:00 ` Claude review: " Claude Code Review Bot
2026-03-03 3:00 ` Claude Code Review Bot
2 siblings, 1 reply; 5+ messages in thread
From: Gary Guo @ 2026-03-02 17:31 UTC (permalink / raw)
To: Tamir Duberstein, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Breno Leitao, David Airlie,
Simona Vetter, Brendan Higgins, David Gow, Rae Moar,
Peter Zijlstra, Ingo Molnar, Will Deacon, Waiman Long
Cc: rust-for-linux, linux-kernel, dri-devel, linux-kselftest,
kunit-dev, Greg Kroah-Hartman
On Mon Mar 2, 2026 at 5:20 PM GMT, Tamir Duberstein wrote:
> Now that all literals are C-Strings, rename and update the documentation
> of this macro to clarify its intended purpose.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1075
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Tamir Duberstein <tamird@kernel.org>
Do we really need to perform the rename? I think updating the documentation
should be good enough?
If the only concern is misuse of the macro then perhaps I can just add a
lint for this in klint.
> ---
> This patch completes the work of replacing our custom `CStr` with
> upstream's.
> ---
> rust/kernel/bug.rs | 2 +-
> rust/kernel/configfs.rs | 2 +-
> rust/kernel/drm/ioctl.rs | 2 +-
> rust/kernel/kunit.rs | 3 ++-
> rust/kernel/str.rs | 19 ++++++++++++++-----
> rust/kernel/sync.rs | 4 ++--
> rust/kernel/sync/lock/global.rs | 3 ++-
> rust/kernel/workqueue.rs | 6 +++---
> 8 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs
> index ed943960f851..f7cb673b1766 100644
> --- a/rust/kernel/bug.rs
> +++ b/rust/kernel/bug.rs
> @@ -80,7 +80,7 @@ macro_rules! warn_flags {
> // with a valid null-terminated string.
> unsafe {
> $crate::bindings::warn_slowpath_fmt(
> - $crate::c_str!(::core::file!()).as_char_ptr(),
> + $crate::str_to_cstr!(::core::file!()).as_char_ptr(),
> line!() as $crate::ffi::c_int,
> $flags as $crate::ffi::c_uint,
> ::core::ptr::null(),
> diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
> index 2339c6467325..930f17bb2041 100644
> --- a/rust/kernel/configfs.rs
> +++ b/rust/kernel/configfs.rs
> @@ -1000,7 +1000,7 @@ macro_rules! configfs_attrs {
> $crate::configfs::Attribute<$attr, $data, $data> =
> unsafe {
> $crate::configfs::Attribute::new(
> - $crate::c_str!(::core::stringify!($name)),
> + $crate::str_to_cstr!(::core::stringify!($name)),
> )
> };
> )*
> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> index cf328101dde4..6a87f489dc88 100644
> --- a/rust/kernel/drm/ioctl.rs
> +++ b/rust/kernel/drm/ioctl.rs
> @@ -157,7 +157,7 @@ macro_rules! declare_drm_ioctls {
> },
> flags: $flags,
> name: $crate::str::as_char_ptr_in_const_context(
> - $crate::c_str!(::core::stringify!($cmd)),
> + $crate::str_to_cstr!(::core::stringify!($cmd)),
> ),
> }
> ),*];
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index f93f24a60bdd..5802a3507ecc 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -59,7 +59,8 @@ macro_rules! kunit_assert {
>
> static FILE: &'static $crate::str::CStr = $file;
> static LINE: i32 = ::core::line!() as i32 - $diff;
> - static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
> + static CONDITION: &'static $crate::str::CStr =
> + $crate::str_to_cstr!(stringify!($condition));
>
> // SAFETY: FFI call without safety requirements.
> let kunit_test = unsafe { $crate::bindings::kunit_get_current_test() };
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index fa87779d2253..8bb40de007d4 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -376,19 +376,28 @@ fn as_ref(&self) -> &BStr {
> }
> }
>
> -/// Creates a new [`CStr`] from a string literal.
> +/// Creates a new [`CStr`] at compile time.
> ///
> -/// The string literal should not contain any `NUL` bytes.
> +/// Rust supports C string literals since Rust 1.77, and they should be used instead of this macro
> +/// where possible. This macro exists to allow static *non-literal* C strings to be created at
> +/// compile time. This is most often used in other macros.
> +///
> +/// # Panics
> +///
> +/// This macro panics if the operand contains an interior `NUL` byte.
> ///
> /// # Examples
> ///
> /// ```
> -/// # use kernel::c_str;
> +/// # use kernel::str_to_cstr;
> /// # use kernel::str::CStr;
> -/// const MY_CSTR: &CStr = c_str!("My awesome CStr!");
> +/// const MY_CSTR: &CStr = str_to_cstr!(concat!(file!(), ":", line!(), ": My CStr!"));
Perhaps keep a copy of the old example and add a comment saying this is allowed
but c"literal" should be preferred.
Best,
Gary
> /// ```
> #[macro_export]
> -macro_rules! c_str {
> +macro_rules! str_to_cstr {
> + // NB: we could write `($str:lit) => compile_error!("use a C string literal instead");` here but
> + // that would trigger when the literal is at the top of several macro expansions. That would be
> + // too limiting to macro authors, so we rely on the name as a hint instead.
> ($str:expr) => {{
> const S: &str = concat!($str, "\0");
> const C: &$crate::str::CStr = match $crate::str::CStr::from_bytes_with_nul(S.as_bytes()) {
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 993dbf2caa0e..ecf02a67ec35 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -154,9 +154,9 @@ macro_rules! static_lock_class {
> #[macro_export]
> macro_rules! optional_name {
> () => {
> - $crate::c_str!(::core::concat!(::core::file!(), ":", ::core::line!()))
> + $crate::str_to_cstr!(::core::concat!(::core::file!(), ":", ::core::line!()))
> };
> ($name:literal) => {
> - $crate::c_str!($name)
> + $crate::str_to_cstr!($name)
> };
> }
> diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
> index aecbdc34738f..81f46229be7d 100644
> --- a/rust/kernel/sync/lock/global.rs
> +++ b/rust/kernel/sync/lock/global.rs
> @@ -272,7 +272,8 @@ macro_rules! global_lock {
> $pub enum $name {}
>
> impl $crate::sync::lock::GlobalLockBackend for $name {
> - const NAME: &'static $crate::str::CStr = $crate::c_str!(::core::stringify!($name));
> + const NAME: &'static $crate::str::CStr =
> + $crate::str_to_cstr!(::core::stringify!($name));
> type Item = $valuety;
> type Backend = $crate::global_lock_inner!(backend $kind);
>
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index 706e833e9702..7616d71df68e 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -212,7 +212,7 @@ macro_rules! new_delayed_work {
> $crate::workqueue::DelayedWork::new(
> $crate::optional_name!(),
> $crate::static_lock_class!(),
> - $crate::c_str!(::core::concat!(
> + $crate::str_to_cstr!(::core::concat!(
> ::core::file!(),
> ":",
> ::core::line!(),
> @@ -223,9 +223,9 @@ macro_rules! new_delayed_work {
> };
> ($name:literal) => {
> $crate::workqueue::DelayedWork::new(
> - $crate::c_str!($name),
> + $crate::str_to_cstr!($name),
> $crate::static_lock_class!(),
> - $crate::c_str!(::core::concat!($name, "_timer")),
> + $crate::str_to_cstr!(::core::concat!($name, "_timer")),
> $crate::static_lock_class!(),
> )
> };
>
> ---
> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
> change-id: 20260302-cstr-rename-macro-64201be6c969
>
> Best regards,
> --
> Tamir Duberstein <tamird@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rust: rename `kernel::c_str!` to `str_to_cstr!`
2026-03-02 17:31 ` Gary Guo
@ 2026-03-02 17:45 ` Tamir Duberstein
0 siblings, 0 replies; 5+ messages in thread
From: Tamir Duberstein @ 2026-03-02 17:45 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Breno Leitao, David Airlie, Simona Vetter, Brendan Higgins,
David Gow, Rae Moar, Peter Zijlstra, Ingo Molnar, Will Deacon,
Waiman Long, rust-for-linux, linux-kernel, dri-devel,
linux-kselftest, kunit-dev, Greg Kroah-Hartman
On Mon, Mar 2, 2026 at 12:31 PM Gary Guo <gary@garyguo.net> wrote:
>
> On Mon Mar 2, 2026 at 5:20 PM GMT, Tamir Duberstein wrote:
> > Now that all literals are C-Strings, rename and update the documentation
> > of this macro to clarify its intended purpose.
> >
> > Link: https://github.com/Rust-for-Linux/linux/issues/1075
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> > Signed-off-by: Tamir Duberstein <tamird@kernel.org>
>
> Do we really need to perform the rename? I think updating the documentation
> should be good enough?
>
> If the only concern is misuse of the macro then perhaps I can just add a
> lint for this in klint.
That would be better! I'll change v2 to only update the documentation.
>
> > ---
> > This patch completes the work of replacing our custom `CStr` with
> > upstream's.
> > ---
> > rust/kernel/bug.rs | 2 +-
> > rust/kernel/configfs.rs | 2 +-
> > rust/kernel/drm/ioctl.rs | 2 +-
> > rust/kernel/kunit.rs | 3 ++-
> > rust/kernel/str.rs | 19 ++++++++++++++-----
> > rust/kernel/sync.rs | 4 ++--
> > rust/kernel/sync/lock/global.rs | 3 ++-
> > rust/kernel/workqueue.rs | 6 +++---
> > 8 files changed, 26 insertions(+), 15 deletions(-)
> >
> > diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs
> > index ed943960f851..f7cb673b1766 100644
> > --- a/rust/kernel/bug.rs
> > +++ b/rust/kernel/bug.rs
> > @@ -80,7 +80,7 @@ macro_rules! warn_flags {
> > // with a valid null-terminated string.
> > unsafe {
> > $crate::bindings::warn_slowpath_fmt(
> > - $crate::c_str!(::core::file!()).as_char_ptr(),
> > + $crate::str_to_cstr!(::core::file!()).as_char_ptr(),
> > line!() as $crate::ffi::c_int,
> > $flags as $crate::ffi::c_uint,
> > ::core::ptr::null(),
> > diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
> > index 2339c6467325..930f17bb2041 100644
> > --- a/rust/kernel/configfs.rs
> > +++ b/rust/kernel/configfs.rs
> > @@ -1000,7 +1000,7 @@ macro_rules! configfs_attrs {
> > $crate::configfs::Attribute<$attr, $data, $data> =
> > unsafe {
> > $crate::configfs::Attribute::new(
> > - $crate::c_str!(::core::stringify!($name)),
> > + $crate::str_to_cstr!(::core::stringify!($name)),
> > )
> > };
> > )*
> > diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
> > index cf328101dde4..6a87f489dc88 100644
> > --- a/rust/kernel/drm/ioctl.rs
> > +++ b/rust/kernel/drm/ioctl.rs
> > @@ -157,7 +157,7 @@ macro_rules! declare_drm_ioctls {
> > },
> > flags: $flags,
> > name: $crate::str::as_char_ptr_in_const_context(
> > - $crate::c_str!(::core::stringify!($cmd)),
> > + $crate::str_to_cstr!(::core::stringify!($cmd)),
> > ),
> > }
> > ),*];
> > diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> > index f93f24a60bdd..5802a3507ecc 100644
> > --- a/rust/kernel/kunit.rs
> > +++ b/rust/kernel/kunit.rs
> > @@ -59,7 +59,8 @@ macro_rules! kunit_assert {
> >
> > static FILE: &'static $crate::str::CStr = $file;
> > static LINE: i32 = ::core::line!() as i32 - $diff;
> > - static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
> > + static CONDITION: &'static $crate::str::CStr =
> > + $crate::str_to_cstr!(stringify!($condition));
> >
> > // SAFETY: FFI call without safety requirements.
> > let kunit_test = unsafe { $crate::bindings::kunit_get_current_test() };
> > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> > index fa87779d2253..8bb40de007d4 100644
> > --- a/rust/kernel/str.rs
> > +++ b/rust/kernel/str.rs
> > @@ -376,19 +376,28 @@ fn as_ref(&self) -> &BStr {
> > }
> > }
> >
> > -/// Creates a new [`CStr`] from a string literal.
> > +/// Creates a new [`CStr`] at compile time.
> > ///
> > -/// The string literal should not contain any `NUL` bytes.
> > +/// Rust supports C string literals since Rust 1.77, and they should be used instead of this macro
> > +/// where possible. This macro exists to allow static *non-literal* C strings to be created at
> > +/// compile time. This is most often used in other macros.
> > +///
> > +/// # Panics
> > +///
> > +/// This macro panics if the operand contains an interior `NUL` byte.
> > ///
> > /// # Examples
> > ///
> > /// ```
> > -/// # use kernel::c_str;
> > +/// # use kernel::str_to_cstr;
> > /// # use kernel::str::CStr;
> > -/// const MY_CSTR: &CStr = c_str!("My awesome CStr!");
> > +/// const MY_CSTR: &CStr = str_to_cstr!(concat!(file!(), ":", line!(), ": My CStr!"));
>
> Perhaps keep a copy of the old example and add a comment saying this is allowed
> but c"literal" should be preferred.
Done in v2.
>
> Best,
> Gary
Thanks for having a look!
Tamir
^ permalink raw reply [flat|nested] 5+ messages in thread
* Claude review: rust: rename `kernel::c_str!` to `str_to_cstr!`
2026-03-02 17:20 [PATCH] rust: rename `kernel::c_str!` to `str_to_cstr!` Tamir Duberstein
2026-03-02 17:31 ` Gary Guo
@ 2026-03-03 3:00 ` Claude Code Review Bot
2026-03-03 3:00 ` Claude Code Review Bot
2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 3:00 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness:** The rename is comprehensive. All 11 occurrences of `c_str!` across 8 files are updated:
- `rust/kernel/bug.rs` (1 occurrence)
- `rust/kernel/configfs.rs` (1)
- `rust/kernel/drm/ioctl.rs` (1)
- `rust/kernel/kunit.rs` (1)
- `rust/kernel/str.rs` (macro definition + doc example)
- `rust/kernel/sync.rs` (2 - both `optional_name!` arms)
- `rust/kernel/sync/lock/global.rs` (1)
- `rust/kernel/workqueue.rs` (3)
No call sites are missed. There are no out-of-tree consumers in the kernel tree.
**Documentation:** The new doc comment is a clear improvement — it explains why the macro exists alongside C string literals and documents the panic condition:
```rust
/// Creates a new [`CStr`] at compile time.
///
/// Rust supports C string literals since Rust 1.77, and they should be used instead of this macro
/// where possible. This macro exists to allow static *non-literal* C strings to be created at
/// compile time. This is most often used in other macros.
```
The updated example using `concat!(file!(), ":", line!(), ": My CStr!")` nicely illustrates the non-literal use case, which is much better pedagogically than the old `c_str!("My awesome CStr!")` example that would now be an anti-pattern.
**Minor nit:** The NB comment uses `$str:lit`:
```rust
// NB: we could write `($str:lit) => compile_error!("use a C string literal instead");` here but
```
The valid Rust macro fragment specifier is `literal`, not `lit` (i.e., `$str:literal`). This is just a comment, so it has no functional impact, but it could confuse someone trying to understand or act on this note.
**Design note:** The `optional_name!($name:literal)` arm now calls `str_to_cstr!($name)` with a literal, which might seem contradictory to the macro's new stated purpose. However, this is correct — even though `$name` is a literal *to the calling macro*, the `optional_name!` macro itself cannot use `c"..."` syntax since `$name` is a metavariable, not a literal token in the macro body. The NB comment in the macro definition explicitly acknowledges this tradeoff.
**No backwards-compatibility shim:** The patch does not leave a `c_str!` alias or deprecation stub. This is fine for an in-kernel macro with no external consumers, and avoids the clutter of a compatibility shim.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread* Claude review: rust: rename `kernel::c_str!` to `str_to_cstr!`
2026-03-02 17:20 [PATCH] rust: rename `kernel::c_str!` to `str_to_cstr!` Tamir Duberstein
2026-03-02 17:31 ` Gary Guo
2026-03-03 3:00 ` Claude review: " Claude Code Review Bot
@ 2026-03-03 3:00 ` Claude Code Review Bot
2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 3:00 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: rust: rename `kernel::c_str!` to `str_to_cstr!`
Author: Tamir Duberstein <tamird@kernel.org>
Patches: 3
Reviewed: 2026-03-03T13:00:33.239684
---
This is a single-patch rename of the `kernel::c_str!` macro to `kernel::str_to_cstr!`. The motivation is that since Rust 1.77, C string literals (`c"..."`) are the preferred way to create `&CStr` values from string literals, and this macro now only exists for non-literal cases (e.g., wrapping `stringify!()`, `concat!()`, `file!()` in macro contexts). The rename makes its narrower intended purpose explicit.
The patch is clean, mechanical, and complete. All 8 call sites within `rust/kernel/` are updated, which matches the full set of files found via grep. The macro is not in the prelude and has no external consumers in-tree (drivers, samples, etc.), so nothing is missed. The updated documentation clearly describes why the macro still exists and when to use it versus C string literals.
Already has Acked-by from Greg Kroah-Hartman and Reviewed-by from Alice Ryhl.
**Verdict:** This looks correct and ready to merge with one very minor nit.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-03 3:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-02 17:20 [PATCH] rust: rename `kernel::c_str!` to `str_to_cstr!` Tamir Duberstein
2026-03-02 17:31 ` Gary Guo
2026-03-02 17:45 ` Tamir Duberstein
2026-03-03 3:00 ` Claude review: " Claude Code Review Bot
2026-03-03 3:00 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox