From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: vc,fbcon,fonts: Proper handling of font data Date: Tue, 10 Mar 2026 12:19:42 +1000 Message-ID: In-Reply-To: <20260309141723.137364-1-tzimmermann@suse.de> References: <20260309141723.137364-1-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: vc,fbcon,fonts: Proper handling of font data Author: Thomas Zimmermann Patches: 15 Reviewed: 2026-03-10T12:19:42.444013 --- This is a well-structured 13-patch series from Thomas Zimmermann that refactors font data handling across VT, fbcon, and newport_con. The series introduces a proper `font_data_t` abstraction with `struct font_data` as the backing storage, replaces ad-hoc reference counting with `font_data_get()`/`font_data_put()`, and consolidates duplicated font import/export code into `lib/fonts/fonts.c`. The progression from trivial prep patches to structural changes is logical. The overall direction is good and the series addresses real problems with the existing fragile, open-coded font handling. However, there are several issues worth raising, mostly around the `font_data_t` typedef design choice and some reference counting edge cases. **Key concerns:** 1. The `typedef const unsigned char font_data_t` approach is unconventional and results in code that immediately casts away the const qualifier when writing to allocated font data. A proper opaque struct pointer would be cleaner. 2. The reference counting uses plain `int` increments/decrements without any atomicity, which is fine if the console lock serializes all access, but this assumption is not documented. 3. The `font_data_export()` function changes the behavior for 17-24 pixel wide fonts (pitch 3), which the commit message acknowledges but could be a user-visible regression. --- --- Generated by Claude Code Patch Reviewer