Skip to content

Conversation

@opsiff
Copy link
Member

@opsiff opsiff commented Dec 26, 2025

deepin inclusion
category: bugfix

charcount in cjk 1616 and cjk 3232 is 65536 * 2, so do this logic to map the source,
c_utf = utf8_pos(vc, s);
if (ch == 0xff)
src = fontdata + (c_utf * cellsize_utf * 2);
else
src = fontdata + (c_utf * cellsize_utf * 2 + cellsize_utf);

Fixes: 458d795 ("fbdev: bitblit: bound-check glyph index in bit_putcs*")

Summary by Sourcery

Fix font data selection and glyph index handling for UTF-8 CJK framebuffer console rendering to avoid out-of-bounds access.

Bug Fixes:

  • Correct glyph source pointer calculation for 16x16 and 32x32 CJK fonts in fbdev bitblit path when using UTF-8 pseudo-characters 0xff/0xfe.
  • Ensure font buffer is initialized before CJK-specific bounds-checked access and fall back to glyph 0 when font data is unavailable.

@sourcery-ai
Copy link

sourcery-ai bot commented Dec 26, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts fontdata initialization and CJK Unicode glyph indexing in fbdev bitblit font_bits() to avoid out-of-bounds access while preserving proper handling of special 0xff/0xfe glyphs.

Flow diagram for updated font_bits CJK glyph handling

flowchart TD
  A_start["font_bits called with vc, s, cellsize, charmask, charcnt, ops"] --> B_readch["read ch from s and apply charmask"]
  B_readch --> C_initfontdata["fontdata = ops.fontbuffer if ops else vc.vc_font.data"]
  C_initfontdata --> D_chk_special["is ch 0xff or 0xfe?"]

  D_chk_special -- yes --> E_charcnt["is charcnt < 65536?"]
  E_charcnt -- yes --> F_has_fontname["have fontname from vc? (vc_utf and vc_encoding)" ]
  F_has_fontname -- yes --> G_find_font["font = find_font(fontname)"]
  G_find_font --> H_set_fontdata["fontdata = font.data if font and font.data else NULL"]
  F_has_fontname -- no --> I_skip_fontlookup["keep existing fontdata"]

  H_set_fontdata --> J_check_fontdata["fontdata is non NULL?"]
  I_skip_fontlookup --> J_check_fontdata
  E_charcnt -- no --> J_check_fontdata

  J_check_fontdata -- yes --> K_utfpos["c_utf = utf8_pos(vc, s)"]
  K_utfpos --> L_ch_ff["is ch 0xff?"]
  L_ch_ff -- yes --> M_src_ff["src = fontdata + c_utf * cellsize_utf * 2"]
  L_ch_ff -- no --> N_src_fe["src = fontdata + c_utf * cellsize_utf * 2 + cellsize_utf"]
  M_src_ff --> O_return_cjk["return src"]
  N_src_fe --> O_return_cjk

  J_check_fontdata -- no --> P_set_ch0["set ch = 0 (fallback glyph)"]

  D_chk_special -- no --> Q_ch_bounds["is ch >= charcnt?"]
  P_set_ch0 --> R_final_src["src = fontdata + ch * cellsize"]
  Q_ch_bounds -- yes --> S_set_ch0_bounds["set ch = 0 (fallback glyph)"]
  Q_ch_bounds -- no --> R_final_src
  S_set_ch0_bounds --> R_final_src

  R_final_src --> T_return["return src"]
Loading

File-Level Changes

Change Details Files
Initialize fontdata earlier and reuse it consistently in font_bits() for both CJK and regular glyph paths.
  • Move fontdata initialization from the generic glyph path to the top of font_bits() so it is available before handling CJK special cases.
  • Ensure both CJK (0xff/0xfe) and non-CJK code paths use the same fontdata pointer when computing the glyph source address.
drivers/video/fbdev/core/bitblit.c
Refine CJK special-case handling to compute UTF-8 glyph positions only when fontdata is valid and to safely fall back when it is not.
  • Limit CJK handling to cases where charcnt indicates a Unicode-capable font and only call utf8_pos() and compute CJK glyph offsets if fontdata is non-NULL.
  • Return the computed CJK glyph pointer immediately on success, and otherwise normalize ch to 0 to safely fall back to the default glyph within bounds.
  • Preserve existing behavior for out-of-range characters by clamping ch to 0 when ch >= charcnt, then computing src based on the validated index.
drivers/video/fbdev/core/bitblit.c

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from opsiff. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个git diff进行详细审查:

  1. 语法逻辑分析:
  • 代码整体逻辑是正确的,主要改动是将fontdata的初始化提前到函数开始处
  • 调整了if-else语句的结构,使代码逻辑更清晰
  • 修改后的代码减少了重复的fontdata赋值操作
  1. 代码质量改进:
  • 优点:
    • 将fontdata的初始化提前,避免了在多个分支中重复赋值
    • 代码结构更加清晰,减少了嵌套层级
    • 统一了错误处理逻辑(当fontdata为空时,将ch设置为0)
  1. 代码性能改进:
  • 性能提升:
    • 减少了重复的fontdata赋值操作
    • 简化了条件判断逻辑,减少了不必要的分支跳转
    • 提前初始化fontdata可以避免后续的重复判断
  1. 代码安全改进:
  • 安全性提升:
    • 增加了对ops指针的空值检查(ops ? ops->fontbuffer : vc->vc_font.data)
    • 保持了原有的字符边界检查(ch >= charcnt)
    • 对非法字符值(0xff, 0xfe)的处理更加严谨
  1. 其他建议:
  • 可以考虑添加对fontdata指针的空值检查注释,说明其在不同场景下的处理逻辑
  • 可以考虑将cellsize_utf的计算也提前,避免在多个分支中重复计算
  • 建议在函数开始处添加参数有效性检查,比如vc和s指针的空值检查

总的来说,这个改动是正面的,提高了代码的可读性和执行效率,同时保持了原有的功能完整性。建议在后续版本中可以考虑上述提到的其他改进点。

deepin inclusion
category: bugfix

charcount in cjk 16*16 and cjk 32*32 is 65536 * 2,
so do this logic to map the source,
c_utf = utf8_pos(vc, s);
if (ch == 0xff)
	src = fontdata + (c_utf * cellsize_utf * 2);
else
	src = fontdata + (c_utf * cellsize_utf * 2 + cellsize_utf);

Fixes: 458d795 ("fbdev: bitblit: bound-check glyph index in bit_putcs*")
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
@opsiff opsiff force-pushed the linux-6.6.y-2025-12-26-cjkttyfix branch from b067537 to 2d955ab Compare December 26, 2025 08:08
@opsiff
Copy link
Member Author

opsiff commented Dec 26, 2025

/changelog edit fixes tag

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants