From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: rust: module: add `THIS_MODULE` const to `ModuleMetadata` trait
Date: Mon, 25 May 2026 20:45:43 +1000
Message-ID: <review-patch1-20260521-fix-fops-owner-v2-1-fd99079c5a04@linux.dev>
In-Reply-To: <20260521-fix-fops-owner-v2-1-fd99079c5a04@linux.dev>
References: <20260521-fix-fops-owner-v2-0-fd99079c5a04@linux.dev> <20260521-fix-fops-owner-v2-1-fd99079c5a04@linux.dev>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

This patch adds `const THIS_MODULE: ThisModule` to the `ModuleMetadata` tra=
it, moves the `THIS_MODULE` static from the `module!` macro body into the `=
ModuleMetadata` impl for the module type, and updates `__init` to use the t=
rait const.

The key change in `rust/kernel/lib.rs`:
```rust
const THIS_MODULE: ThisModule;
```

And in `rust/macros/module.rs`, the old `static THIS_MODULE` (with `#[cfg(M=
ODULE)]` / `#[cfg(not(MODULE))]` variants) becomes:
```rust
impl ::kernel::ModuleMetadata for #type_ {
    const NAME: &'static ::kernel::str::CStr =3D #name_cstr;

    #[cfg(MODULE)]
    const THIS_MODULE: ::kernel::ThisModule =3D {
        extern "C" {
            static __this_module: ::kernel::types::Opaque<::kernel::binding=
s::module>;
        }
        unsafe { ::kernel::ThisModule::from_ptr(__this_module.get()) }
    };

    #[cfg(not(MODULE))]
    const THIS_MODULE: ::kernel::ThisModule =3D unsafe {
        ::kernel::ThisModule::from_ptr(::core::ptr::null_mut())
    };
}
```

**Observations:**

1. **`static` =E2=86=92 `const` semantics for `THIS_MODULE`**: The old `sta=
tic THIS_MODULE` had a stable address. Making it a `const` means each use s=
ite can potentially get its own copy. However, `ThisModule` is just a wrapp=
er around `*mut module` =E2=80=94 a pointer-sized value =E2=80=94 so this s=
hould be fine. The pointer *value* (i.e., the address of `__this_module`) i=
s the important thing, not the address of the `ThisModule` wrapper itself. =
Callers that need a `&ThisModule` reference (like `__init`) will take a ref=
erence to a temporary, which is valid in Rust for const promotion. This is =
correct.

2. The `SAFETY` comment is preserved and appropriate =E2=80=94 `__this_modu=
le` is kernel-constructed and lives for the module lifetime. The `from_ptr(=
null_mut())` path for built-in is consistent with the previous behavior.

3. The `__init` call site change from `&super::super::THIS_MODULE` to `&<su=
per::super::LocalModule as ::kernel::ModuleMetadata>::THIS_MODULE` is verbo=
se but correct =E2=80=94 the fully-qualified syntax avoids ambiguity.

**No issues found.**

---
Generated by Claude Code Patch Reviewer