From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/atomic: Add error_code element in atomic_state
Date: Wed, 01 Apr 2026 07:55:27 +1000
Message-ID: <review-patch2-20260331-atomic-v11-2-6a1df7ec5af8@intel.com>
In-Reply-To: <20260331-atomic-v11-2-6a1df7ec5af8@intel.com>
References: <20260331-atomic-v11-0-6a1df7ec5af8@intel.com> <20260331-atomic-v11-2-6a1df7ec5af8@intel.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Memory leak in `drm_mode_atomic_add_error_msg()`:**
```c
failure_string = kvasprintf(GFP_ATOMIC, format, varg);
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);
```
`kvasprintf()` allocates memory via `kmalloc` but there is no `kfree(failure_string)` anywhere. This leaks on every error. Add `kfree(failure_string)` after `strscpy_pad`, or better yet, use `vsnprintf` directly into `err_code->failure_string` to avoid the allocation entirely.

**No NULL check on `kvasprintf` return:** `kvasprintf` can return NULL on allocation failure, which would then be passed to `drm_dbg_atomic` and `strscpy_pad`, causing a NULL dereference.

**`container_of` usage is fragile:** The function takes `struct drm_mode_atomic_err_code *err_code` and uses `container_of` to get back to `struct drm_atomic_state`. This only works if `err_code` is always `&state->error_code`. If someone ever passes a standalone struct, this will produce garbage. Consider just passing `struct drm_atomic_state *state` directly.

**Kernel-doc says "Returns: void"** -- kernel-doc convention doesn't require documenting void returns. Minor nit.

**Comment references wrong enum name:**
```c
/* @error_code: pointer to struct holding failure reason and string
 *
 * Error codes defined in enum drm_mode_atomic_failure_flags
 */
```
The enum is called `drm_mode_atomic_failure_codes`, not `drm_mode_atomic_failure_flags`. Also, this comment should use `/**` kernel-doc format since it's documenting a struct member.

---

---
Generated by Claude Code Patch Reviewer