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/mode-config: Create drm_mode_config_create_initial_state()
Date: Sat, 16 May 2026 13:22:55 +1000
Message-ID: <review-patch16-20260512-drm-mode-config-init-v4-16-591dfdcc1bf9@kernel.org>
In-Reply-To: <20260512-drm-mode-config-init-v4-16-591dfdcc1bf9@kernel.org>
References: <20260512-drm-mode-config-init-v4-0-591dfdcc1bf9@kernel.org> <20260512-drm-mode-config-init-v4-16-591dfdcc1bf9@kernel.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

**Bug: connector list iterator leaked on error.**

In `drm_mode_config_create_initial_state()`:

```c
drm_connector_list_iter_begin(dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
    if (connector->state)
        continue;

    ret =3D drm_mode_config_connector_create_state(connector);
    if (ret)
        return ret;  // <-- BUG: misses drm_connector_list_iter_end()
}
drm_connector_list_iter_end(&conn_iter);
```

If `drm_mode_config_connector_create_state()` returns an error, the functio=
n returns without calling `drm_connector_list_iter_end(&conn_iter)`. The it=
erator holds a reference that won't be released. This needs to be fixed, e.=
g.:

```c
    if (ret) {
        drm_connector_list_iter_end(&conn_iter);
        return ret;
    }
```

Additionally, there's **no cleanup of already-allocated states on error**. =
If, say, plane states are created successfully but a CRTC state allocation =
fails, the already-created plane states remain allocated and dangling. Whet=
her this matters depends on whether the caller tears down the device on fai=
lure =E2=80=94 the skeleton in patch 17 shows `return ret` which would trig=
ger `drmm_mode_config_init` cleanup via devres, which should handle it. But=
 it's worth considering explicitly.

Also, the function skips colorop `atomic_create_state` =E2=80=94 it calls `=
drm_atomic_helper_colorop_create_state()` directly, which means drivers can=
not override colorop state creation. This is consistent since colorops don'=
t have an `atomic_create_state` callback in `drm_colorop_funcs` (the callba=
ck added in patch 7 is a free function, not a vtable entry). Just noting th=
e asymmetry.

---
Generated by Claude Code Patch Reviewer