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: Compare font data for equality with font_data_is_equal() Date: Tue, 03 Mar 2026 13:19:15 +1000 Message-ID: In-Reply-To: <20260302141255.518657-10-tzimmermann@suse.de> References: <20260302141255.518657-1-tzimmermann@suse.de> <20260302141255.518657-10-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 Introduces `font_data_is_equal()` and `font_data_is_internal()` (using `is_= kernel_rodata()`). The equality check is well-designed =E2=80=94 it prevents comparing interna= l fonts with user-space fonts (which would break reference counting). Docum= entation typos: - `"is constain the same sequence"` should be `"it contains the same sequen= ce"` - `"Right-hand-size"` should be `"Right-hand-side"` Note: `is_kernel_rodata()` only covers vmlinux `.rodata`, not module `.roda= ta`. If fonts were ever built as modules, `font_data_is_internal()` would r= eturn false for module-resident font data, and `font_data_get()/put()` woul= d attempt to modify read-only module data. The font Kconfig options appear = to be `bool` (not `tristate`), so this should be safe today, but it's a fra= gile assumption. A comment documenting this constraint would be helpful. The removal of the `tmp->vc_font.width =3D=3D w` check from `fbcon_set_font= ()` is intentional per the commit message =E2=80=94 the width is implicitly= part of the data layout. This seems correct since two fonts with the same = byte content but different widths would have different glyph renderings, bu= t the data comparison would match. However, in practice, this can't happen = because fonts with different widths have different pitches and thus differe= nt data layouts. --- Generated by Claude Code Patch Reviewer