From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/gem: Try to fix change_handle ioctl, attempt 4 Date: Fri, 05 Jun 2026 06:00:21 +1000 Message-ID: In-Reply-To: <20260604191916.1713387-1-simona.vetter@ffwll.ch> References: <20260604191916.1713387-1-simona.vetter@ffwll.ch> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: drm/gem: Try to fix change_handle ioctl, attempt 4 Author: Simona Vetter Patches: 3 Reviewed: 2026-06-05T06:00:21.529702 --- This mbox contains two versions (v1 and v2) of the same single patch. The v= 2 supersedes v1, with the key change being the use of `IS_ERR_OR_NULL()` in= stead of `!obj` for the `idr_replace()` return value check =E2=80=94 a crit= ical fix since `idr_replace()` returns `ERR_PTR(-ENOENT)` for non-existent = IDs, not NULL. The patch addresses a genuinely tangled mess: three stacked bugfix commits = (`5e28b7b94408`, `7164d78559b0`, `dc366607c41c`) each partially wrong due t= o confusing variable naming (`handle` actually meant `new_handle`). The app= roach is sound =E2=80=94 it rebuilds the function using the two established= patterns from the same file: 1. **`drm_gem_handle_delete`'s pattern**: `idr_replace(NULL, handle)` to at= omically grab the object and NULL out the old slot 2. **`drm_gem_handle_create_tail`'s pattern**: `idr_alloc(NULL, ...)` then = later `idr_replace(obj, ...)` for the two-stage allocation Disabling the ioctl via `drm_invalid_op` until IGT tests and consensus are = in place is a reasonable defensive measure given the history of repeated in= correct fixes. The v2 patch is **correct** and significantly cleaner than the existing cod= e. I have only minor observations below. --- Generated by Claude Code Patch Reviewer