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/i915/gt: Use IS_ERR() check for kernel_context() in selftests Date: Sat, 16 May 2026 10:29:02 +1000 Message-ID: In-Reply-To: <20260514193227.2472962-1-ingyujang25@korea.ac.kr> References: <20260514193227.2472962-1-ingyujang25@korea.ac.kr> <20260514193227.2472962-1-ingyujang25@korea.ac.kr> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Correctness of the core fix: Good.** The commit message accurately descri= bes the problem: `kernel_context()` returns an error pointer on failure, so= `!ctx` checks are dead code that won't catch allocation failures. The fix = is the right pattern. **Four call sites fixed correctly:** 1. **`live_preempt()`** (line 1744): `ctx_hi` =E2=80=94 straightforward ear= ly return, correct. 2. **`live_preempt()`** (line 1749): `ctx_lo` =E2=80=94 correctly captures = `PTR_ERR(ctx_lo)` into `err` before `goto err_ctx_hi`, since the previous c= ode was relying on the `err =3D -ENOMEM` initializer which would have been = wrong for a different error code. 3. **`live_late_preempt()`** (lines 1838, 1842): Same pattern, correct. 4. **`preempt_client_init()`** (line 1940): Direct return of `PTR_ERR`, cor= rect. 5. **`live_preempt_timeout()`** (lines 3390, 3395): Same pattern as `live_p= reempt`, correct. **Issue: Missed call site in the same file.** At `selftest_execlists.c:3685`: ```c smoke.contexts[n] =3D kernel_context(smoke.gt->i915, NULL); if (!smoke.contexts[n]) goto err_ctx; ``` This has the identical bug but was not fixed. The `err_ctx` cleanup path it= erates `smoke.contexts[]` and calls `kernel_context_close()` on each, so st= oring an `ERR_PTR` there would also cause a crash during cleanup. The fix h= ere requires both converting the check to `IS_ERR()` and clearing the point= er (or otherwise guarding the cleanup loop). Since the commit message claim= s to fix this pattern in the file, this omission should be addressed. **Minor style note:** The added braces around single-statement `goto` block= s (e.g., `if (IS_ERR(ctx_lo)) { err =3D PTR_ERR(ctx_lo); goto err_ctx_hi; }= `) are correct since these are now multi-statement blocks (assignment + got= o). **Recommendation:** Request a v2 that also fixes the `live_preempt_smoke()`= site at line 3685, and ideally audits other selftest files for the same pa= ttern. --- Generated by Claude Code Patch Reviewer