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: gpuvm: add GpuVa struct Date: Sun, 22 Mar 2026 03:35:24 +1000 Message-ID: In-Reply-To: <20260320-gpuvm-rust-v5-4-76fd44f17a87@google.com> References: <20260320-gpuvm-rust-v5-0-76fd44f17a87@google.com> <20260320-gpuvm-rust-v5-4-76fd44f17a87@google.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 Introduces `GpuVa`, `GpuVaAlloc`, and `GpuVaRemoved`. **SAFETY comment typo in `obj()`:** ```rust /// Returns the GEM object. pub fn obj(&self) -> &T::Object { // SAFETY: The `gem.offset` field of `drm_gpuva` is immutable. unsafe { ::from_raw((*self.as_raw()).gem.ob= j) } } ``` The SAFETY comment says "gem.offset" but it's accessing `gem.obj`. Should s= ay "gem.obj". **`GpuVaAlloc::prepare` consumes self but leaks memory on panic:** ```rust pub(super) fn prepare(mut self, va_data: impl PinInit) -> *mut b= indings::drm_gpuva { let va_ptr =3D MaybeUninit::as_mut_ptr(&mut self.0); let Ok(()) =3D unsafe { va_data.__pinned_init(&raw mut (*va_ptr).data) = }; KBox::into_raw(self.0).cast() } ``` The `let Ok(()) =3D` pattern on an infallible init is fine. But `self` is c= onsumed by move, and `KBox::into_raw` forgets the box =E2=80=94 the caller = takes ownership of the raw pointer. This is correct for the intended usage = where the pointer is passed to `drm_gpuva_map`/`drm_gpuva_remap`. **Missing `GpuVaAlloc` destructor:** If a `GpuVaAlloc` is dropped (e.g., th= e `unused_va` in `OpRemapRet`), the `KBox>>` is droppe= d which frees the allocation. However, since `MaybeUninit` doesn't run dest= ructors and the memory was zeroed (not initialized), this is actually corre= ct =E2=80=94 no `T::VaData` destructor runs because none was initialized. G= ood. --- Generated by Claude Code Patch Reviewer