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/mode-config: Create drm_mode_config_create_state() Date: Wed, 11 Mar 2026 13:09:08 +1000 Message-ID: In-Reply-To: <20260310-drm-mode-config-init-v1-8-de7397c8e1cf@kernel.org> References: <20260310-drm-mode-config-init-v1-0-de7397c8e1cf@kernel.org> <20260310-drm-mode-config-init-v1-8-de7397c8e1cf@kernel.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Bug: Missing `drm_connector_list_iter_end()` on error path.** In `drm_mode_config_create_state()`: ```c drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter) { if (connector->state) continue; ret = drm_mode_config_connector_create_state(connector); if (ret) return ret; // <-- leaks the connector iterator! } drm_connector_list_iter_end(&conn_iter); ``` If `drm_mode_config_connector_create_state()` fails, the function returns without calling `drm_connector_list_iter_end()`, which will leak the connector list iteration reference. This needs a `goto` to a cleanup label. Additionally, the function has **no cleanup on partial failure** for any of the earlier loops either. If e.g. crtc state allocation fails after plane states were successfully allocated, those plane states are leaked. The function probably needs either full rollback or should be documented as leaving partial state that the caller must handle. The `// TODO: Implement atomic_create_state for colorop.` comment with a fallback to `drm_colorop_reset()` is fine for now but should be tracked. The doc update to the state lifetime documentation is good, clearly separating "initialization time" from "reset time". --- Generated by Claude Code Patch Reviewer