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/vmwgfx: Add some checks to vmw_cursor_plane_atomic_update Date: Thu, 04 Jun 2026 13:30:07 +1000 Message-ID: In-Reply-To: <20260602004203.102901-2-maaz.mombasawala@broadcom.com> References: <20260602004203.102901-1-maaz.mombasawala@broadcom.com> <20260602004203.102901-2-maaz.mombasawala@broadcom.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 **Status: Looks good, minor style nit** The fix correctly addresses a null dereference when `old_state` is not pres= ent. The root cause is that the old code did: ```c struct drm_crtc *crtc =3D new_state->crtc ?: old_state->crtc; struct vmw_display_unit *du =3D vmw_crtc_to_du(crtc); ``` ...which crashes if both are NULL or `old_state` itself is NULL. The new approach of getting `du` via `container_of(plane, struct vmw_displa= y_unit, cursor.base)` is correct =E2=80=94 the cursor plane *is* `du->curso= r.base`, so `container_of` is valid and avoids the dependency on any plane = state. The early return on `!new_state` is also appropriate since there's n= othing to update. **Minor style nit**: The declarations of `dev_priv` and `du` appear after t= he `if (!new_state) return;` statement: ```c new_state =3D drm_atomic_get_new_plane_state(state, plane); if (!new_state) return; struct vmw_private *dev_priv =3D vmw_priv(plane->dev); struct vmw_display_unit *du =3D vmw_cursor_to_du(plane); ``` While this is valid C11 (which the kernel uses), it's a coding style prefer= ence issue. Some reviewers prefer all declarations before statements within= a block. Not a correctness issue. --- --- Generated by Claude Code Patch Reviewer