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/nouveau: handle nouveau_hw_get_clock() return value Date: Sat, 16 May 2026 10:30:37 +1000 Message-ID: In-Reply-To: <20260514174008.4515-1-ms.bulatov7@gmail.com> References: <20260514174008.4515-1-ms.bulatov7@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/nouveau: handle nouveau_hw_get_clock() return value Author: Bulatov Mark Patches: 1 Reviewed: 2026-05-16T10:30:37.685317 --- This is a single-patch series addressing a potential division-by-zero in th= e nouveau NV04 display arbitration code. The problem is real: `nouveau_hw_g= et_clock()` can return 0 (when `nouveau_hw_pllvals_to_clk()` gets zero M1/M= 2 divisors) or a negative error code (when `nouveau_hw_get_pllvals()` fails= with `-ENOENT`). These values are then used as divisors throughout `nv04_c= alc_arb()` and `nv10_calc_arb()`, so the defensive check is warranted. However, the fix has a significant problem: it introduces a **silent early = return that leaves the caller's output parameters (`*burst` and `*lwm`) uni= nitialized**. The caller in `crtc.c:893` declares `arb_burst` and `arb_lwm`= as bare `int` locals (line 848) without initialization, then immediately w= rites them to CRTC registers after the call. A silent return turns a divisi= on-by-zero into use-of-uninitialized-stack-data written to hardware registe= rs =E2=80=94 arguably trading one bug for another. **Verdict: Needs revision.** The issue being fixed is real, but the fix nee= ds to either (a) initialize the output parameters to safe defaults before r= eturning, or (b) propagate an error code so the caller can skip the registe= r writes. --- Generated by Claude Code Patch Reviewer