From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: rust: rename `kernel::c_str!` to `str_to_cstr!` Date: Tue, 03 Mar 2026 13:00:33 +1000 Message-ID: In-Reply-To: <20260302-cstr-rename-macro-v1-1-a269fe4dc3f0@kernel.org> References: <20260302-cstr-rename-macro-v1-1-a269fe4dc3f0@kernel.org> <20260302-cstr-rename-macro-v1-1-a269fe4dc3f0@kernel.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 =E2=80=94 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 use= d instead of this macro /// where possible. This macro exists to allow static *non-literal* C strin= gs to be created at /// compile time. This is most often used in other macros. ``` The updated example using `concat!(file!(), ":", line!(), ": My CStr!")` ni= cely illustrates the non-literal use case, which is much better pedagogical= ly than the old `c_str!("My awesome CStr!")` example that would now be an a= nti-pattern. **Minor nit:** The NB comment uses `$str:lit`: ```rust // NB: we could write `($str:lit) =3D> compile_error!("use a C string l= iteral instead");` here but ``` The valid Rust macro fragment specifier is `literal`, not `lit` (i.e., `$st= r: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 =E2=80=94 even though `$name`= is a literal *to the calling macro*, the `optional_name!` macro itself can= not use `c"..."` syntax since `$name` is a metavariable, not a literal toke= n in the macro body. The NB comment in the macro definition explicitly ackn= owledges this tradeoff. **No backwards-compatibility shim:** The patch does not leave a `c_str!` al= ias or deprecation stub. This is fine for an in-kernel macro with no extern= al consumers, and avoids the clutter of a compatibility shim. --- Generated by Claude Code Patch Reviewer