From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: rust: drm: gem: shmem: Add DmaResvGuard helper Date: Thu, 04 Jun 2026 11:29:38 +1000 Message-ID: In-Reply-To: <20260603195210.693856-3-lyude@redhat.com> References: <20260603195210.693856-1-lyude@redhat.com> <20260603195210.693856-3-lyude@redhat.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Verdict: Minor concerns.** The RAII guard pattern is correct. However: **Return value of `dma_resv_lock()` is discarded:** ```rust fn new(obj: &'a Object) -> 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` (it wraps `ww_mutex_lock()`). With `ctx = NULL`, it degrades to a plain mutex lock that always returns 0, so this is safe in practice -- and matches the C-side pattern in `drm_gem_shmem_release()` which also discards the return. But the safety comment should mention *why* the return value is ignored (i.e., NULL ctx guarantees success). Future readers won't necessarily know this. **The `#[expect(unused)]` is removed in patch 3** -- fine, but introducing a dead-code function in one patch just to use it in the next is slightly awkward for bisection. The function has no callers between patches 2 and 3. Not a blocker. --- --- Generated by Claude Code Patch Reviewer