[PATCH] rust: kunit: use C-string literals to clean warning

[PATCH] rust: kunit: use C-string literals to clean warning

[Miguel Ojeda]

Starting with upstream Rust commit a5e3a3f9b6bd ("move
`manual_c_str_literals` to complexity"), to be released in Rust 1.83.0
[1], Clippy now warns on `manual_c_str_literals` by default, e.g.:

    error: manually constructing a nul-terminated string
      --> rust/kernel/kunit.rs:21:13
       |
    21 |             b"\x013%pA\0".as_ptr() as _,
       |             ^^^^^^^^^^^^^ help: use a `c""` literal: `c"\x013%pA"`
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
       = note: `-D clippy::manual-c-str-literals` implied by `-D warnings`
       = help: to override `-D warnings` add `#[allow(clippy::manual_c_str_literals)]`

Apply the suggestion to clean up the warnings.

Link: https://github.com/rust-lang/rust-clippy/pull/13263 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
The cast could also be cleaned up, but that is already done in Gary's
FFI series, so I didn't include another patch.

C-string literals are available since Rust 1.77.0, which is higher than
our minimum version, but LTS kernels do not have Rust 1.77.0. But
perhaps for these is not a big deal, and we could also enable
`feature(c_str_literals)` instead.

 rust/kernel/kunit.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 0ba77276ae7e..824da0e9738a 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -18,7 +18,7 @@ pub fn err(args: fmt::Arguments<'_>) {
     #[cfg(CONFIG_PRINTK)]
     unsafe {
         bindings::_printk(
-            b"\x013%pA\0".as_ptr() as _,
+            c"\x013%pA".as_ptr() as _,
             &args as *const _ as *const c_void,
         );
     }
@@ -34,7 +34,7 @@ pub fn info(args: fmt::Arguments<'_>) {
     #[cfg(CONFIG_PRINTK)]
     unsafe {
         bindings::_printk(
-            b"\x016%pA\0".as_ptr() as _,
+            c"\x016%pA".as_ptr() as _,
             &args as *const _ as *const c_void,
         );
     }

base-commit: 570172569238c66a482ec3eb5d766cc9cf255f69
--
2.46.2

Re: [PATCH] rust: kunit: use C-string literals to clean warning

[Benno Lossin]

On 27.09.24 18:44, Miguel Ojeda wrote:
> Starting with upstream Rust commit a5e3a3f9b6bd ("move
> `manual_c_str_literals` to complexity"), to be released in Rust 1.83.0
> [1], Clippy now warns on `manual_c_str_literals` by default, e.g.:
> 
>     error: manually constructing a nul-terminated string
>       --> rust/kernel/kunit.rs:21:13
>        |
>     21 |             b"\x013%pA\0".as_ptr() as _,
>        |             ^^^^^^^^^^^^^ help: use a `c""` literal: `c"\x013%pA"`
>        |
>        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
>        = note: `-D clippy::manual-c-str-literals` implied by `-D warnings`
>        = help: to override `-D warnings` add `#[allow(clippy::manual_c_str_literals)]`
> 
> Apply the suggestion to clean up the warnings.
> 
> Link: https://github.com/rust-lang/rust-clippy/pull/13263 [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

> ---
> The cast could also be cleaned up, but that is already done in Gary's
> FFI series, so I didn't include another patch.
> 
> C-string literals are available since Rust 1.77.0, which is higher than

I thought our minimal version is 1.78.0, or am I misunderstanding the
wording here (I interpret higher as "1.77 > min-version").

---
Cheers,
Benno

> our minimum version, but LTS kernels do not have Rust 1.77.0. But
> perhaps for these is not a big deal, and we could also enable
> `feature(c_str_literals)` instead.
> 
>  rust/kernel/kunit.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 0ba77276ae7e..824da0e9738a 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -18,7 +18,7 @@ pub fn err(args: fmt::Arguments<'_>) {
>      #[cfg(CONFIG_PRINTK)]
>      unsafe {
>          bindings::_printk(
> -            b"\x013%pA\0".as_ptr() as _,
> +            c"\x013%pA".as_ptr() as _,
>              &args as *const _ as *const c_void,
>          );
>      }
> @@ -34,7 +34,7 @@ pub fn info(args: fmt::Arguments<'_>) {
>      #[cfg(CONFIG_PRINTK)]
>      unsafe {
>          bindings::_printk(
> -            b"\x016%pA\0".as_ptr() as _,
> +            c"\x016%pA".as_ptr() as _,
>              &args as *const _ as *const c_void,
>          );
>      }
> 
> base-commit: 570172569238c66a482ec3eb5d766cc9cf255f69
> --
> 2.46.2

Re: [PATCH] rust: kunit: use C-string literals to clean warning

[Miguel Ojeda]

On Fri, Sep 27, 2024 at 7:06 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> I thought our minimal version is 1.78.0, or am I misunderstanding the
> wording here (I interpret higher as "1.77 > min-version").

Yeah, sorry, I meant that our minimum is higher and thus we can use
the feature in mainline, but not in LTSs -- rewording artifact :)

Cheers,
Miguel

Re: [PATCH] rust: kunit: use C-string literals to clean warning

[Gary Guo]

On Fri, 27 Sep 2024 18:44:14 +0200
Miguel Ojeda <ojeda@kernel.org> wrote:

> Starting with upstream Rust commit a5e3a3f9b6bd ("move
> `manual_c_str_literals` to complexity"), to be released in Rust 1.83.0
> [1], Clippy now warns on `manual_c_str_literals` by default, e.g.:
> 
>     error: manually constructing a nul-terminated string
>       --> rust/kernel/kunit.rs:21:13  
>        |
>     21 |             b"\x013%pA\0".as_ptr() as _,
>        |             ^^^^^^^^^^^^^ help: use a `c""` literal: `c"\x013%pA"`
>        |
>        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
>        = note: `-D clippy::manual-c-str-literals` implied by `-D warnings`
>        = help: to override `-D warnings` add `#[allow(clippy::manual_c_str_literals)]`
> 
> Apply the suggestion to clean up the warnings.
> 
> Link: https://github.com/rust-lang/rust-clippy/pull/13263 [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> The cast could also be cleaned up, but that is already done in Gary's
> FFI series, so I didn't include another patch.

Note that the cast actually need to be re-introduced instead of
cleaned-up -- `c"".as_ptr()` returns `core::ffi::c_char` which might be
signed, while after my series `*const u8` is expected by `printf`.

An alternative is to use `c_str!(...).as_char_ptr()` which will
guarantee the type is what we expected.

Best,
Gary

> 
> C-string literals are available since Rust 1.77.0, which is higher than
> our minimum version, but LTS kernels do not have Rust 1.77.0. But
> perhaps for these is not a big deal, and we could also enable
> `feature(c_str_literals)` instead.
> 
>  rust/kernel/kunit.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 0ba77276ae7e..824da0e9738a 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -18,7 +18,7 @@ pub fn err(args: fmt::Arguments<'_>) {
>      #[cfg(CONFIG_PRINTK)]
>      unsafe {
>          bindings::_printk(
> -            b"\x013%pA\0".as_ptr() as _,
> +            c"\x013%pA".as_ptr() as _,
>              &args as *const _ as *const c_void,
>          );
>      }
> @@ -34,7 +34,7 @@ pub fn info(args: fmt::Arguments<'_>) {
>      #[cfg(CONFIG_PRINTK)]
>      unsafe {
>          bindings::_printk(
> -            b"\x016%pA\0".as_ptr() as _,
> +            c"\x016%pA".as_ptr() as _,
>              &args as *const _ as *const c_void,
>          );
>      }
> 
> base-commit: 570172569238c66a482ec3eb5d766cc9cf255f69
> --
> 2.46.2

Re: [PATCH] rust: kunit: use C-string literals to clean warning

[Trevor Gross]

On Fri, Sep 27, 2024 at 12:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Starting with upstream Rust commit a5e3a3f9b6bd ("move
> `manual_c_str_literals` to complexity"), to be released in Rust 1.83.0
> [1], Clippy now warns on `manual_c_str_literals` by default, e.g.:
>
>     error: manually constructing a nul-terminated string
>       --> rust/kernel/kunit.rs:21:13
>        |
>     21 |             b"\x013%pA\0".as_ptr() as _,
>        |             ^^^^^^^^^^^^^ help: use a `c""` literal: `c"\x013%pA"`
>        |
>        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
>        = note: `-D clippy::manual-c-str-literals` implied by `-D warnings`
>        = help: to override `-D warnings` add `#[allow(clippy::manual_c_str_literals)]`
>
> Apply the suggestion to clean up the warnings.
>
> Link: https://github.com/rust-lang/rust-clippy/pull/13263 [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

Re: [PATCH] rust: kunit: use C-string literals to clean warning

[Miguel Ojeda]

On Fri, Sep 27, 2024 at 6:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Starting with upstream Rust commit a5e3a3f9b6bd ("move
> `manual_c_str_literals` to complexity"), to be released in Rust 1.83.0
> [1], Clippy now warns on `manual_c_str_literals` by default, e.g.:
>
>     error: manually constructing a nul-terminated string
>       --> rust/kernel/kunit.rs:21:13
>        |
>     21 |             b"\x013%pA\0".as_ptr() as _,
>        |             ^^^^^^^^^^^^^ help: use a `c""` literal: `c"\x013%pA"`
>        |
>        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
>        = note: `-D clippy::manual-c-str-literals` implied by `-D warnings`
>        = help: to override `-D warnings` add `#[allow(clippy::manual_c_str_literals)]`
>
> Apply the suggestion to clean up the warnings.
>
> Link: https://github.com/rust-lang/rust-clippy/pull/13263 [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Applied to `rust-fixes` -- thanks everyone!

Cheers,
Miguel

Re: [PATCH] rust: kunit: use C-string literals to clean warning

[Miguel Ojeda]

On Sat, Sep 28, 2024 at 8:36 PM Gary Guo <gary@garyguo.net> wrote:
>
> Note that the cast actually need to be re-introduced instead of
> cleaned-up -- `c"".as_ptr()` returns `core::ffi::c_char` which might be
> signed, while after my series `*const u8` is expected by `printf`.

Ah, right, in your series `bindgen` will point to the new one while
the method will return the `core::ffi` one which might be still `i8`
in some targets.

It would be nice to get a way to modify the target for `u8`, i.e.
`-Cunsigned-char` or a "target modifier" or similar.

Added an entry with some links in our `rustc` list at
https://github.com/Rust-for-Linux/linux/issues/355.

Cheers,
Miguel