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/atomic: Add error_code element in atomic_state Date: Wed, 11 Feb 2026 16:31:47 +1000 Message-ID: In-Reply-To: <20260210-atomic-v9-2-525c88fd2402@intel.com> References: <20260210-atomic-v9-0-525c88fd2402@intel.com> <20260210-atomic-v9-2-525c88fd2402@intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Mailer: Claude Code Patch Reviewer Patch Review **Issues:** ```c +void drm_mode_atomic_add_error_msg(struct drm_mode_atomic_err_code *err_code, + __u64 failure_code, const char *format, ...) +{ + struct drm_atomic_state *state = container_of(err_code, + struct drm_atomic_state, + error_code); ``` - **Questionable Design**: Using `container_of` to get back to `drm_atomic_state` is fragile. Why not pass `state` directly? ```c + va_list varg; + char *failure_string; + + err_code->failure_code = failure_code; + + va_start(varg, format); + failure_string = kvasprintf(GFP_ATOMIC, format, varg); ``` - **Memory Leak**: `failure_string` is allocated with `kvasprintf` but never freed - **NULL Check Missing**: `kvasprintf` can return NULL, not checked before use - **GFP_ATOMIC**: Why GFP_ATOMIC? The ioctl path should allow GFP_KERNEL ```c + drm_dbg_atomic(state->dev, "Commit failed: %s\n", failure_string); + strscpy_pad(err_code->failure_string, failure_string, + sizeof(err_code->failure_string)); + va_end(varg); +} ``` - **Use After Free Potential**: If `failure_string` is NULL (allocation failed), this crashes - **Missing kfree**: Memory leak confirmed --- Generated by Claude Code Patch Reviewer