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/amdgpu: switch XGMI sysfs show helpers to sysfs_emit_at() Date: Sat, 14 Mar 2026 07:16:57 +1000 Message-ID: In-Reply-To: <20260313015226.39727-1-davidbaum461@gmail.com> References: <20260313015226.39727-1-davidbaum461@gmail.com> <20260313015226.39727-1-davidbaum461@gmail.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 **Positive aspects:** 1. Fixes a real (albeit likely benign in practice) undefined behavior issue= where the old code passes `buf` as both output buffer and `%s` format argu= ment in `sysfs_emit(buf, "%s\n", buf)`. 2. The conversion to `sysfs_emit_at()` is the correct modern kernel API for= building up sysfs output incrementally. 3. Matches the existing style of `amdgpu_xgmi_show_connected_port_num()` at= line 505 of the same file, which already uses `sysfs_emit_at()` with a `si= ze` offset variable. **Minor concern:** The return value calculation on the final line: ```c return offset + sysfs_emit_at(buf, offset, "\n"); ``` This works correctly =E2=80=94 `offset` holds the bytes written so far, and= `sysfs_emit_at()` returns the number of bytes written for the newline (1),= so the total returned is the full length. However, there is an edge case: = if `sysfs_emit_at()` were to return a negative error value, this addition w= ould produce an incorrect result. In practice, `sysfs_emit_at()` only retur= ns negative if `offset` is already beyond `PAGE_SIZE`, which won't happen h= ere with a reasonable `num_nodes` count. The neighboring `amdgpu_xgmi_show_= connected_port_num()` uses the same `size +=3D` pattern, so this is consist= ent. **Overall:** The patch is clean, correct, and well-scoped. No issues that w= ould block merging. Reviewed-by: looks good to me. --- Generated by Claude Code Patch Reviewer