Re: [PATCH V8 00/14] Rust bindings for cpufreq and OPP core + sample driver

[Miguel Ojeda]

On Fri, Feb 7, 2025 at 8:15 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> If I move the code as suggested here, then I get warning about not
> adding a SAFETY comment for unsafe code (which looks to be a tool
> specific bug).

The warning is there even if you don't run `rustfmt`, and it does not
look like a bug to me -- what Clippy is complaining about is that you
don't actually need the `unsafe` block to begin with:

    error: unnecessary `unsafe` block
    --> rust/kernel/cpufreq.rs:631:22
        |
    631 |         attr[next] = unsafe {
        |                      ^^^^^^ unnecessary `unsafe` block
        |
        = note: `-D unused-unsafe` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(unused_unsafe)]`

since those operations are safe. Or am I missing something?

Then, when you remove it, Clippy will complain that there should not
be a SAFETY comment:

    error: statement has unnecessary safety comment
    --> rust/kernel/cpufreq.rs:625:9
        |
    625 |         attr[next] =
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as
*mut _;
        |
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        |
    help: consider removing the safety comment
    --> rust/kernel/cpufreq.rs:623:9
        |
    623 |         // SAFETY: The C code returns a valid pointer here,
which is again passed to the C code in
        |
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment
        = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings`
        = help: to override `-D warnings` add
`#[allow(clippy::unnecessary_safety_comment)]`

And `rustfmt` will put things in a single line, since now they fit.

I would suggest reviewing all the SAFETY comments around this code,
i.e. something may be wrong, since these were not needed, and thus you
may have wanted to describe them elsewhere.

In any case, passing `rustfmtcheck` is a requirement. So in the worst
case, if you do find such a bug in e.g. Clippy, you may always
`expect` or `allow` the lint or disable `rustfmt` in that region of
code. But that should be really rare, and in such a case it should be
reported upstream.

I also found other build issues in the branch you mention in your
cover letter, so please double-check everything looks good before
adding it to linux-next. Please also make it Clippy-clean.

I hope that helps!

Cheers,
Miguel