From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: lib/fonts: Manage font-data lifetime with font_data_get/_put() Date: Tue, 03 Mar 2026 13:19:15 +1000 Message-ID: In-Reply-To: <20260302141255.518657-11-tzimmermann@suse.de> References: <20260302141255.518657-1-tzimmermann@suse.de> <20260302141255.518657-11-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review This is a key patch that introduces proper reference counting via `font_dat= a_get()`/`font_data_put()` and removes the `userfont` flag from fbcon. The `to_font_data_struct()` using `container_of(fd, struct font_data, data[= 0])` is correct but unusual =E2=80=94 `container_of` on a flexible array me= mber's first element. This works because the macro just does pointer arithm= etic. `font_data_put()` returns `true` for internal fonts: ```c if (font_data_is_internal(fd)) return true; /* never unref static data */ ``` This is semantically confusing =E2=80=94 `true` means "final reference, dat= a freed" but internal fonts are never freed. No callers check the return va= lue, so it's not a bug, but the comment could be improved. The WARN_ON checks in get/put for zero refcount are good defensive programm= ing: ```c if (WARN_ON(!REFCOUNT(fd))) return; /* should never be 0 */ ``` In `newport_set_def_font()`: ```c font_data_put(font_data[unit]); font_data[unit] =3D FONT_DATA; font_data_get(font_data[unit]); ``` The `font_data_get(FONT_DATA)` call is a no-op since FONT_DATA is internal.= This is correct but slightly wasteful =E2=80=94 an asymmetry where `put` r= eleases the old data but `get` on internal data does nothing. It's fine for= correctness. --- Generated by Claude Code Patch Reviewer