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: drm: gem: shmem: Add DmaResvGuard helper
Date: Fri, 05 Jun 2026 06:03:37 +1000
Message-ID: <review-patch1-20260604192740.659240-2-lyude@redhat.com>
In-Reply-To: <20260604192740.659240-2-lyude@redhat.com>
References: <20260604192740.659240-1-lyude@redhat.com> <20260604192740.659240-2-lyude@redhat.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Bug =E2=80=94 return value of `dma_resv_lock` is ignored:**

```rust
fn new(obj: &'a Object<T, C>) -> Self {
    // SAFETY: This lock is initialized throughout the lifetime of `object`.
    unsafe { bindings::dma_resv_lock(obj.raw_dma_resv(), ptr::null_mut()) };
    Self(obj)
}
```

`dma_resv_lock()` returns `int`. With `ctx` as NULL, `ww_mutex_lock` should=
 succeed (no deadlock detection context), but it can still return `-EALREAD=
Y` if the caller already holds the lock. Ignoring the return value means on=
 failure, the `Drop` impl will call `dma_resv_unlock()` on a lock that was =
never acquired =E2=80=94 undefined behavior. At minimum, `new()` should ret=
urn `Result<Self>` and propagate the error.

**Minor =E2=80=94 `#[expect(unused)]` on `new()`:** This attribute is remov=
ed in patch 2, which is fine, but the helper itself is dead code in this pa=
tch. Consider squashing patches 1 and 2 since the guard has no users until =
patch 2.

**Comment nit:** The TODO comment about WwMutex is good forward-looking doc=
umentation.

---
Generated by Claude Code Patch Reviewer