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: Look up glyph data with font_data_glyph_buf() Date: Thu, 04 Jun 2026 16:27:41 +1000 Message-ID: In-Reply-To: <20260529140759.529929-2-tzimmermann@suse.de> References: <20260529140759.529929-1-tzimmermann@suse.de> <20260529140759.529929-2-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 patch adds two functions to `lib/fonts/fonts.c`: ```c static unsigned int font_data_num_glyphs(font_data_t *fd, unsigned int widt= h, unsigned int height) { return font_data_size(fd) / font_glyph_size(width, height); } ``` ```c const unsigned char *font_data_glyph_buf(font_data_t *fd, unsigned int width, unsigned int vpitch, unsigned int c) { if (c >=3D font_data_num_glyphs(fd, width, vpitch)) return NULL; return font_data_buf(fd) + font_glyph_size(width, vpitch) * c; } ``` I verified that internal built-in fonts correctly set up `FNTSIZE` in their= `extra` array (e.g. `font_8x16.c` uses `{ 0, 0, FONTDATAMAX, 0 }` where `F= ONTDATAMAX=3D4096`, matching `256 glyphs * 16 bytes`), so `font_data_size()= ` will return the right value and `font_data_num_glyphs()` computes correct= ly. **Minor observations:** - The parameter naming is slightly inconsistent: `font_data_num_glyphs` cal= ls its third parameter `height`, but `font_data_glyph_buf` calls the same p= arameter `vpitch`. Both mean "number of scanlines per glyph." The doc comme= nt on `font_data_glyph_buf` correctly says "@vpitch: The number of scanline= s per glyph" so this is just cosmetic, but using `vpitch` consistently woul= d be cleaner. - If `width` or `height` were ever 0, `font_glyph_size()` returns 0 and `fo= nt_data_num_glyphs()` divides by zero. This can't happen with valid font da= ta and the existing code already assumes non-zero dimensions everywhere, so= this is not a real concern =E2=80=94 just noting there's no explicit guard. - The kdoc is good and clearly documents the NULL return case. **No bugs. Looks good.** --- Generated by Claude Code Patch Reviewer