Skip to content

クリップボード独自形式の長さフィールドを32bit固定に戻す - #2557

Open
beru wants to merge 2 commits into
sakura-editor:masterfrom
beru:clipboard_copy_INT_MAX_compatibility
Open

クリップボード独自形式の長さフィールドを32bit固定に戻す#2557
beru wants to merge 2 commits into
sakura-editor:masterfrom
beru:clipboard_copy_INT_MAX_compatibility

Conversation

@beru

@beru beru commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

自分が #2067 で入れた不具合を放置していてすみません。@hpmy-dev さんが修正PR(#2450, #2453)を作ってくれたのですが差分内容がちょっと過大に感じたので別途PRを作成しました。単純に 2b921e0 をrevertで済む問題ではなかったのでClaude Codeに修正してもらいました。

PR対象

  • アプリ(サクラエディタ本体)
  • テストコード

カテゴリ

  • 不具合修正

PR の背景

#2325

仕様・動作説明

SAKURAClipW形式の先頭に置く文字数フィールドの型がsize_tになっており、x64/ARM64 ビルドでは8バイト、Win32ビルドでは4バイトとレイアウトが食い違っていた。このため異なるビット数のビルド間や過去バージョンとの間でコピー&ペーストするとデータ位置が4バイトずれ、文字化けやクラッシュを起こしていた。

長さフィールドの型をint32_t固定のSAKURAClipW_LengthFieldTypeとして定義し直し、 クリップボードとドラッグ&ドロップの読み書きで使う。表現できない長さのときは独自形式を設定せずCF_UNICODETEXTのみとする。読み込み側にはGlobalSizeによる上限クランプを 追加し、壊れたデータや他ビルドが書き込んだデータを読んでも確保領域外を参照しないようにした。

テスト内容

32bit版と64bit版を両方起動して相互でコピペが問題無く行える事を確認した。

関連 issue, PR

#2067 #2325 #2331 #2435 #2453 #2556

@beru
beru requested review from berryzplus and m-tmatma July 26, 2026 20:05
@beru beru added the 🐛bug🦋 ■バグ修正(Something isn't working) label Jul 26, 2026
@github-actions

Copy link
Copy Markdown

Test Results

1 202 tests  +4   1 202 ✅ +4   4m 4s ⏱️ -1s
  107 suites ±0       0 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 4bdd81e. ± Comparison against base commit 6eaf0f4.

@sonarqubecloud

Copy link
Copy Markdown

berryzplus
berryzplus previously approved these changes Jul 26, 2026

@berryzplus berryzplus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

対応ありがとうございます。

以下のフィールド名変更は、そのうち元に戻すと思います。

GlobalSakura::size_type = size_t; //TODO: int32_t に戻す
 ↓ リネーム
GlobalSakura::LengthFieldType

(名前が長い、「より分かりやすい」には見えない。)

@beru

beru commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

以下のフィールド名変更は、そのうち元に戻すと思います。

GlobalSakura::size_type = size_t; //TODO: int32_t に戻す  ↓ リネーム GlobalSakura::LengthFieldType

(名前が長い、「より分かりやすい」には見えない。)

レビューありがとうございます。

STLにもsize_typeがあるので個人的には違う名前が良いです。

@hpmy-dev

Copy link
Copy Markdown
Contributor

ご対応ありがとうございます。

以下 1 点だけ気になりました。

長さフィールドの型が 2 箇所で独立に定義されている

  • sakura_core/_os/CClipboard.h : using SAKURAClipW_LengthFieldType = int32_t;
  • sakura_core/util/os.h L294 : using LengthFieldType = int32_t;

両者が一致していることを保証しているのが現状コメントだけなので、将来どちらか片方だけが書き換えられてもビルドもテストも通ってしまい、異なるビルド間でコピー&ペーストしたユーザーだけが文字化けに遭遇する、という #2325 と同じ壊れ方を再現できてしまいます。本 PR がまさに「同一レイアウトを扱う箇所で型定義がずれていた」ことを直すものなので、統一を検討して頂ければと思いました。

  • 案 1util/os.h から _os/CClipboard.h を include し、using LengthFieldType = CClipboard::SAKURAClipW_LengthFieldType; として定義を一本化する(ただしinclude 依存は増えます)

  • 案 2util/os.cpp は既に _os/CClipboard.h を include しているので、.cpp 側に static_assert(std::is_same_v<LengthFieldType, CClipboard::SAKURAClipW_LengthFieldType>); を追記する

  • 案 3_os/CClipboard.hにもコメントを追記する

m-tmatma
m-tmatma previously approved these changes Jul 27, 2026
@berryzplus

Copy link
Copy Markdown
Contributor

以下のフィールド名変更は、そのうち元に戻すと思います。
GlobalSakura::size_type = size_t; //TODO: int32_t に戻す  ↓ リネーム GlobalSakura::LengthFieldType
(名前が長い、「より分かりやすい」には見えない。)

レビューありがとうございます。

STLにもsize_typeがあるので個人的には違う名前が良いです。

「STLの命名法則嫌い」は分からなくないです。
std::wstring::npos は自分も好きじゃありません。

STLっぽく使う意図を読み取りやすくしておくことで、
説明コメントが要らなくなる効果を狙っていて、
変数名を変えるならたくさん説明を書かないといけない気がします。
(なお、今回対応は不要と思ってます。)

@beru

beru commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

STLにもsize_typeがあるので個人的には違う名前が良いです。

「STLの命名法則嫌い」は分からなくないです。 std::wstring::npos は自分も好きじゃありません。

STLの命名規則については受け入れるしかないし好き嫌いとかは無いんですが、コードの読解をちゃんとしていない状態で GlobalSakura::size_type を見ると、これが何を意味するかを間違えやすいので GlobalSakura::LengthFieldType に変えてしまいました。ただこれでも、GlobalAlloc で確保した領域上の "SAKURAClipW" 形式のヘッダである文字数フィールドの型 っていうのがやっぱり分かりにくいですね。

STLっぽく使う意図を読み取りやすくしておくことで、 説明コメントが要らなくなる効果を狙っていて、 変数名を変えるならたくさん説明を書かないといけない気がします。 (なお、今回対応は不要と思ってます。)

手続きを種別毎に書くやり方じゃなくて型でうまく分けていく書き方に寄せたいという事ですかね? CClipBoard.cpp 側での適用は現時点では出来ていないので、理想と現実との距離を埋めるのは大変そうです。

@beru
beru dismissed stale reviews from m-tmatma and berryzplus via e35978f July 27, 2026 12:59
beru and others added 2 commits July 27, 2026 22:01
SAKURAClipW形式の先頭に置く文字数フィールドの型がsize_tになっており、x64/ARM64
ビルドでは8バイト、Win32ビルドでは4バイトとレイアウトが食い違っていた。このため
異なるビット数のビルド間や過去バージョンとの間でコピー&ペーストするとデータ位置が
4バイトずれ、文字化けやクラッシュを起こしていた。

長さフィールドの型をint32_t固定のSAKURAClipW_LengthFieldTypeとして定義し直し、
クリップボードとドラッグ&ドロップの読み書きで使う。表現できない長さのときは独自形式を
設定せずCF_UNICODETEXTのみとする。読み込み側にはGlobalSizeによる上限クランプを
追加し、壊れたデータや他ビルドが書き込んだデータを読んでも確保領域外を参照しない
ようにした。

sakura-editor#2325

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GlobalSakuraがサクラエディタ独自クリップボード形式SAKURAClipWを扱う
クラスであることがクラス名にもコメントにも書かれておらず、実装を読まないと
用途が分からない状態だった。クラスコメントに対応形式とバイナリレイアウトを
明記し、他のGlobal*クラスにも対応形式を1行で追記する。

また文字数フィールドの型がCClipboard::SAKURAClipW_LengthFieldTypeと
GlobalSakura::LengthFieldTypeの2箇所で独立に定義されており、一致保証が
コメントだけだった。片方だけ書き換えてもビルドもテストも通ってしまい、
issue #2325と同じ壊れ方を再現できるため、CClipboard側の定義を唯一の正とし、
os.cppでそのエイリアスとしてsize_typeを定義する形に改める。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@beru
beru force-pushed the clipboard_copy_INT_MAX_compatibility branch from e35978f to 7a1a3ca Compare July 27, 2026 13:01
@beru

beru commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Approveしていただいた後で申し訳ないですが、レビューコメントに対応する変更を行いました。
再度レビューお願いします。

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

Labels

🐛bug🦋 ■バグ修正(Something isn't working)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants