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: lib/fonts: Store font data as font_data_t; update consoles
Date: Tue, 10 Mar 2026 12:19:44 +1000
Message-ID: <review-patch7-20260309141723.137364-8-tzimmermann@suse.de>
In-Reply-To: <20260309141723.137364-8-tzimmermann@suse.de>
References: <20260309141723.137364-1-tzimmermann@suse.de> <20260309141723.137364-8-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 the core type-system patch. **This patch has the most design concer=
ns.**

The typedef:
```c
typedef const unsigned char font_data_t;
```

This is an unusual choice. `font_data_t *` becomes `const unsigned char *`,=
 yet the code immediately casts away const when writing to allocated font d=
ata:
```c
memset((u8 *)new_data, 0, FONT_EXTRA_WORDS * sizeof(int));
...
memcpy((u8 *)new_data + i * h * pitch, data + i * vpitch * pitch, h * pitch=
);
```

A more conventional approach would be to use an opaque struct as the type (=
e.g., `struct font_data *`), which would prevent accidental direct pointer =
arithmetic while not requiring const-casting. The current approach means th=
at `font_data_buf()` is essentially a no-op identity cast, and all the type=
 safety comes from convention rather than the type system.

The `font_data_buf()` helper:
```c
static inline const unsigned char *font_data_buf(font_data_t *fd)
{
	return (const unsigned char *)fd;
}
```
This returns the same type as its argument. It exists as documentation that=
 the caller wants the raw buffer, but adds no actual type safety.

In `fbcon_set_disp`:
```c
-	if (!vc->vc_font.data) {
+	if (!p->fontdata) {
```
This changes the semantics of the check =E2=80=94 previously it checked `vc=
->vc_font.data`, now it checks `p->fontdata`. These could theoretically dif=
fer. The change looks intentional (the following code sets both from `t->fo=
ntdata`), but worth confirming the two are always in sync at this point.

---
Generated by Claude Code Patch Reviewer