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/blend: size zpos array to active planes instead of total Date: Tue, 05 May 2026 09:43:21 +1000 Message-ID: In-Reply-To: <20260501001301.7666-1-gustavopiazdasilva2102@gmail.com> References: <20260501001301.7666-1-gustavopiazdasilva2102@gmail.com> 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/blend: size zpos array to active planes instead of total Author: Gustavo Piaz Da Silva Patches: 2 Reviewed: 2026-05-05T09:43:21.243447 --- This is a single-patch series that reduces the allocation size of a tempora= ry array in `drm_atomic_helper_crtc_normalize_zpos()` from `num_total_plane= ` (all planes on the device) to `hweight32(crtc_state->plane_mask)` (only t= he active planes on the given CRTC). The change is **correct**. The `drm_for_each_plane_mask()` loop iterates ex= actly over the bits set in `plane_mask`, so `hweight32(plane_mask)` gives t= he precise count of array entries that will be populated. The original allo= cation was safe but conservatively oversized. That said, the practical benefit is minimal. A typical device might have 3-= 8 total planes, and each array entry is a single pointer (8 bytes on 64-bit= ). The savings are on the order of a handful of bytes per CRTC atomic commi= t =E2=80=94 not meaningful "memory pressure." The real value is arguably in= making the code more self-documenting: the allocation size now directly re= flects the iteration count. **One potential issue worth flagging:** `plane_mask` is `u32` (confirmed at= `include/drm/drm_crtc.h:195`), so `hweight32()` is type-correct today. How= ever, if `plane_mask` were ever widened (e.g., to `u64` for devices with >3= 2 planes), the `hweight32()` call would silently truncate and produce an un= dersized allocation, causing an out-of-bounds write at line 492: ```c states[n++] =3D plane_state; ``` This is not a bug today, but it is a latent fragility the original code did= n't have. Using `hweight_long()` or adding a `BUILD_BUG_ON(sizeof(crtc_stat= e->plane_mask) > sizeof(u32))` would harden against that future scenario. --- --- Generated by Claude Code Patch Reviewer