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/syncobj: add drm_syncobj_timeline_signal Date: Mon, 18 May 2026 16:53:31 +1000 Message-ID: In-Reply-To: <20260516-jorth-syncobj-v1-7-88ede9d98a81@gmail.com> References: <20260516-jorth-syncobj-v1-0-88ede9d98a81@gmail.com> <20260516-jorth-syncobj-v1-7-88ede9d98a81@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Design concern**: The exported function signature is: ```c int drm_syncobj_timeline_signal(struct drm_syncobj **syncobjs, u64 user_points, u32 count) ``` `user_points` is a raw `__u64` cast from a user pointer, and the function internally does `copy_from_user(points, u64_to_user_ptr(user_points), ...)`. An exported kernel API function should not take a disguised user pointer as a `u64`. The caller (patch 12's `syncobj_ioctl_signal`) should copy the points array from userspace and pass a kernel pointer. This is the standard kernel pattern -- ioctls copy from user, then call internal functions with kernel pointers. **Bug**: The new function adds `if (count == 0) return -EINVAL;` but the original ioctl code didn't have this check at the function level (it was checked earlier in the ioctl handler via `args->count_handles`). The semantics are preserved for the ioctl path since that check already existed, but it means the exported API differs subtly from what you might expect. --- Generated by Claude Code Patch Reviewer