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/amd/display: Fix a benign uninitialized variable warning Date: Wed, 11 Mar 2026 13:10:19 +1000 Message-ID: In-Reply-To: <6aaf2cf4bd19363a85f35e649685d7bdae400253.1773157137.git.calvin@wbinvd.org> References: <6aaf2cf4bd19363a85f35e649685d7bdae400253.1773157137.git.calvin@wbinvd.org> <6aaf2cf4bd19363a85f35e649685d7bdae400253.1773157137.git.calvin@wbinvd.org> 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** The original code was: ```c adev->dm.cached_state =3D drm_atomic_helper_suspend(adev_to_drm(adev)); if (IS_ERR(adev->dm.cached_state)) { r =3D PTR_ERR(adev->dm.cached_state); adev->dm.cached_state =3D NULL; } return adev->dm.cached_state ? 0 : r; ``` GCC correctly identifies that if `drm_atomic_helper_suspend()` returns a va= lid (non-NULL, non-ERR) pointer, the `IS_ERR` branch is not taken and `r` r= emains uninitialized =E2=80=94 yet the final `return` expression `adev->dm.= cached_state ? 0 : r` would use `r` if `cached_state` were NULL. In practic= e this path is unreachable (a valid pointer is not NULL), but the compiler = can't prove that. The fix replaces this with: ```c r =3D PTR_ERR_OR_ZERO(adev->dm.cached_state); if (r) adev->dm.cached_state =3D NULL; return r; ``` This is logically equivalent and cleaner: - `PTR_ERR_OR_ZERO()` returns 0 for valid pointers and the error code for `= IS_ERR` pointers, so `r` is always initialized. - The `if (r)` branch correctly NULLs out the cached state on error. - `return r` returns 0 on success, the negative error code on failure =E2= =80=94 same behavior as before. **Minor observations:** - The commit message is well-written and honest about the cosmetic nature o= f the fix. - The `int r` declaration could now be combined with initialization (`int r= =3D PTR_ERR_OR_ZERO(...)`) to further simplify, but keeping the declaratio= n separate is fine and matches kernel style in this file. - No functional change, no risk of regression. **Reviewed-by worthy.** --- Generated by Claude Code Patch Reviewer