From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: fbdev: savage: fix probe-path EDID cleanup leaks Date: Thu, 23 Apr 2026 10:23:48 +1000 Message-ID: In-Reply-To: <20260420051926.28276-1-dbgh9129@gmail.com> References: <20260420051926.28276-1-dbgh9129@gmail.com> <20260420051926.28276-1-dbgh9129@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Correctness: Good.** The patch adds the two missing cleanup calls in the = right place: ```c failed: #ifdef CONFIG_FB_SAVAGE_I2C savagefb_delete_i2c_busses(info); + fb_destroy_modelist(&info->modelist); + fb_destroy_modedb(info->monspecs.modedb); #endif ``` Key points verified: 1. **All `goto failed` sites (lines 2265, 2281, 2310) occur after the EDID/= I2C setup block (lines 2221=E2=80=932228)**, so the allocations exist whene= ver we reach the `failed:` label. Earlier failures use distinct labels (`fa= iled_video`, `failed_mmio`, `failed_init`, `failed_enable`). 2. **No double-free risk.** The one path that could cause trouble is line 2= 310 (`register_framebuffer` failure), which is reached after the success pa= th already freed `monspecs.modedb` and set it to NULL at lines 2305=E2=80= =932306. But `fb_destroy_modedb` is just `kfree()` (line 697 of `fbmon.c`),= which is a no-op on NULL. The modelist is NOT freed on the success path be= fore `register_framebuffer`, so `fb_destroy_modelist` is also safe there. 3. **Placement inside `#ifdef CONFIG_FB_SAVAGE_I2C` is correct** since the = allocations only happen under that same guard (lines 2221=E2=80=932228). **Minor observation (not a blocker):** The existing `savagefb_remove()` fun= ction (line 2342) also doesn't call `fb_destroy_modelist()`. On the success= path, `monspecs.modedb` is freed at line 2305, but the modelist entries cr= eated at line 2226 are never freed during normal remove. This is a pre-exis= ting leak unrelated to this patch =E2=80=94 but the author may want to addr= ess it in a follow-up. **Commit message:** Accurate and well-written. Clearly describes the proble= m, the root cause, and the fix. --- Generated by Claude Code Patch Reviewer