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: dma: add generalized container for types other than slices
Date: Sun, 22 Mar 2026 03:23:42 +1000
Message-ID: <review-patch2-20260320194626.36263-3-dakr@kernel.org>
In-Reply-To: <20260320194626.36263-3-dakr@kernel.org>
References: <20260320194626.36263-1-dakr@kernel.org> <20260320194626.36263-3-dakr@kernel.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This is the core refactoring patch. Key observations:

**Typo in commit message**: "concecptually" should be "conceptually" at line 296.

**`as_mut_ptr` on `&self`**: The method at the new code:
```rust
pub fn as_mut_ptr(&self) -> *mut T {
    self.cpu_addr.as_ptr()
}
```
This returns a `*mut T` from `&self`, which is intentional since `Coherent` represents shared DMA memory where the hardware may also be writing. The `as_mut()` method that uses it is marked `unsafe` with appropriate safety docs. This is consistent with the existing pattern but worth noting: the `#[expect(clippy::mut_from_ref)]` annotation on `as_mut()` correctly suppresses the lint.

**Zero-length allocation check**: The `alloc_slice_with_attrs` function correctly bails early for `len == 0`:
```rust
if len == 0 {
    Err(EINVAL)?;
}
```
This is needed because `dma_alloc_attrs` cannot handle zero-length. The use of `?` with `Err()` is unusual but functionally correct (equivalent to `return Err(EINVAL)`).

**`Drop` impl uses `KnownSize`**: The unified `Drop` for `Coherent<T: KnownSize + ?Sized>` uses `T::size(self.cpu_addr.as_ptr())` to compute the deallocation size. This is sound as long as `KnownSize` correctly reports sizes for both sized types and slices, which it should given the `NonNull::slice_from_raw_parts` construction for slices.

**Missing `Sync` impl**: The original had only `Send`, and this patch preserves that. `Sync` is intentionally not implemented since `Coherent` provides interior mutability through raw pointers to DMA memory. This is correct.

---
Generated by Claude Code Patch Reviewer