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/modeset_lock: add NULL check for ctx before WARN_ON Date: Tue, 05 May 2026 11:50:04 +1000 Message-ID: In-Reply-To: <20260429025952.1202080-1-george.d.sworo@intel.com> References: <20260429060431.1462589-1-george.d.sworo@intel.com> <20260429025952.1202080-1-george.d.sworo@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **NAK.** The change: ```c - if (WARN_ON(ctx->contended)) + if (ctx && WARN_ON(ctx->contended)) ``` **Problem 1: The guard is unnecessary.** As discussed above, `modeset_lock()` is a static helper only called from `drm_modeset_lock()` (which already NULL-checks `ctx` before calling it) and `drm_modeset_backoff()` (which dereferences `ctx` before calling it). There is no path where `ctx` is NULL here. **Problem 2: The patch is incomplete even on its own terms.** It guards the `ctx->contended` dereference on line 298, but the very next line that uses `ctx` is: ```c if (ctx->trylock_only) { ``` This dereference at line 301 is left completely unguarded. If `ctx` truly could be NULL, this patch would still crash two lines later. Patch 1 in isolation does not produce a correct result. **Problem 3: Inaccurate commit message.** The commit message says "modeset_lock() and drm_modeset_drop_locks() do not validate the ctx pointer" but neither this patch nor patch 2 modifies `drm_modeset_drop_locks()`. --- Generated by Claude Code Patch Reviewer