Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIS<=>SJIS 変換の際、常に日本語ロケールを使用する #1578

Merged
merged 1 commit into from
Mar 13, 2021

Conversation

k-takata
Copy link
Member

@k-takata k-takata commented Mar 10, 2021

PR の目的

英語版WindowsでEUC-JPのファイルが文字化けするのを修正する。

カテゴリ

  • 不具合修正

PR の背景

#1103 参照。

PR のメリット

英語版Windowsでの文字化けが直る。

PR のデメリット (トレードオフとかあれば)

多少遅くなる可能性あり。
_create_locale() で作成したロケールが解放されない。

仕様・動作説明

サクラエディタでは _mbcjistojms() と _mbcjmstojis() を JIS<=>SJIS の変換に使用していますが、これらの関数は現在のロケールが日本語の時しか動作しません。
代わりに _mbcjistojms_l() と _mbcjmstojis_l() を使い、日本語ロケールを明示的に指定することで、現在のロケールによらず、常に JIS<=>SJIS の変換を行えるようにします。

別案としては、_mbcjistojms() と _mbcjmstojis() からロケールのチェックを省いた関数を自前で実装する方法が考えられます。この場合、ロケールのチェックがない分、高速に動作する可能性がありますが、 変換ロジックでバグが混入するリスクが高まるでしょう。

PR の影響範囲

テスト内容

  • 日本語版Windowsで、EUC-JPのファイルを読み書きして文字化けしない。
  • 日本語版Windowsで、JISのファイルを読み書きして文字化けしない。
  • 英語版Windowsで、EUC-JPのファイルを読み書きして文字化けしない。
  • 英語版Windowsで、JISのファイルを読み書きして文字化けしない。

関連 issue, PR

Fix #1103

参考資料

Fixes sakura-editor#1103

Use _mbcjistojms_l and _mbcjmstojis_l instead of _mbcjistojms and
_mbcjmstojis, so that always use Japanese locale when using these
functions.
@sonarcloud
Copy link

sonarcloud bot commented Mar 10, 2021

@AppVeyorBot
Copy link

Build sakura 1.0.3532 completed (commit e3904ce2ee by @k-takata)

@beru beru added the 🐛bug🦋 ■バグ修正(Something isn't working) label Mar 12, 2021
Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

英語版Windowsでは未確認ですが、#1103 (comment) に書かれている方法でロケールを en-US に変更して確認しました。

EUC-JPのファイルを開くと文字化けする問題が解消される事を確認しました。

@beru
Copy link
Contributor

beru commented Mar 12, 2021

JIS<=>SJIS 変換で CRT の _mbcjistojms_mbcjmstojis を使わずに WindowsAPI の MultiByteToWideCharWideCharToMultiByte の組み合わせで変換するやり方も考えられます。

明示的にコードページ指定すればロケールの影響を受けないで変換出来るかもしれません。
https://docs.microsoft.com/en-us/windows/win32/intl/code-page-identifiers

難点としてはちょびっとだけ実装とテストが面倒な事でしょうか…。

@arigayas
Copy link

知っているかもですが Microsoftのサイトから開発者向けのWindows10の仮想OSイメージ が配布されていて、
たしかそれは英語版のWindows10だった気がします。
なのでこれを使って英語版のテストは出来ると思います。

@beru
Copy link
Contributor

beru commented Mar 12, 2021

知っているかもですが Microsoftのサイトから開発者向けのWindows10の仮想OSイメージ が配布されていて、
たしかそれは英語版のWindows10だった気がします。
なのでこれを使って英語版のテストは出来ると思います。

知らなかったです、前にも教えてもらったかもしれないんですが忘れてました。
リンク先を見てみると下記のように書かれてました。

Windows 10 Enterprise - 20 GB のダウンロード
この VM の有効期限は 2021 年 3 月 14 日です。

落としてもすぐ有効期限になっちゃいそうなのでやめときます。

@k-takata
Copy link
Member Author

JIS<=>SJIS 変換で CRT の _mbcjistojms_mbcjmstojis を使わずに WindowsAPI の MultiByteToWideCharWideCharToMultiByte の組み合わせで変換するやり方も考えられます。

51932 だか 20932 を指定すれば EUC-JP と UTF-16 を、JIS, SJIS を経由せずに変換できて効率が良さそうですが、今と同じ変換をしてくれるのかが分からないですね。(JIS は 50220, 50221, 50222 らしい。) 処理を大幅に変更する必要がありますが。
既存の処理を極力変更せずに修正するなら今回のようになるかなと。

なお、_mbcjistojms_mbcjmstojis のソースは
C:\Program Files (x86)\Windows Kits\10\Source\10.0.18362.0\ucrt\mbstring\tojisjms.cpp
などにあります。上にも書いたように、現在のロケールが日本語かどうかをチェックしてる部分があるので、それを除いたものを自前で用意すれば JIS<=>SJIS 変換はわずかに速くなりそうです。

@beru
Copy link
Contributor

beru commented Mar 13, 2021

51932 だか 20932 を指定すれば EUC-JP と UTF-16 を、JIS, SJIS を経由せずに変換できて効率が良さそうですが、今と同じ変換をしてくれるのかが分からないですね。(JIS は 50220, 50221, 50222 らしい。) 処理を大幅に変更する必要がありますが。
既存の処理を極力変更せずに修正するなら今回のようになるかなと。

確かに元実装と同じ変換がされるか分からないですね。使う手間より確かめる手間のが掛かりそうです。

なお、_mbcjistojms_mbcjmstojis のソースは
C:\Program Files (x86)\Windows Kits\10\Source\10.0.18362.0\ucrt\mbstring\tojisjms.cpp
などにあります。上にも書いたように、現在のロケールが日本語かどうかをチェックしてる部分があるので、それを除いたものを自前で用意すれば JIS<=>SJIS 変換はわずかに速くなりそうです。

教えてくれたソースコードのファイルパスを開いて内容を見てみました。
思っていたより行数が少なくて驚きました(文字コード周りの知識はほぼ無い人の感想です)。

改変したソースコードを配布してよいのかは分からなかったので調べてみました。
https://docs.microsoft.com/en-us/legal/windows-sdk/license-terms-ewdk
iii. Distribution Restrictions. You may not の5番以降に接触するので残念ながら駄目なんじゃないかなと思います。

ところでなんでMSの実装はわざわざロケールのチェックを入れるんですかね?関数名に jis とか jms が含まれているので目的は明確だと思いますが、日本語以外のロケールだと JIS<=>SJIS 変換をされたら困るユースケースがあるんでしょうか?
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/mbcjistojms-mbcjistojms-l-mbcjmstojis-mbcjmstojis-l?view=msvc-160
の記載を見てもそれについては読み取れませんでした。

@k-takata
Copy link
Member Author

k-takata commented Mar 13, 2021

改変したソースコードを配布してよいのかは分からなかったので調べてみました。
https://docs.microsoft.com/en-us/legal/windows-sdk/license-terms-ewdk
iii. Distribution Restrictions. You may not の5番以降に接触するので残念ながら駄目なんじゃないかなと思います。

はい、CRT のコードを丸ごとコピーしてきて使うのはダメだと思います。
やるとしたら、フリーで公開されてる変換コードを持ってきて、変換エラーのチェックを CRT の仕様に合わせるのがよいかと。

@k-takata
Copy link
Member Author

ところでなんでMSの実装はわざわざロケールのチェックを入れるんですかね?

実装上の話で言うと、_mbcjmstojis_l は、変換前のコードが有効かどうかを判定するために _ismbblead_l, _ismbbtrail_l を使っている部分はロケールに依存していますね。
ただ、変換処理自体はロケールに依存しませんし、よく分かりませんね。

@k-takata k-takata marked this pull request as ready for review March 13, 2021 01:54
@k-takata
Copy link
Member Author

approveいただきましたので、マージしたいと思います。

@k-takata k-takata merged commit 1e64509 into sakura-editor:master Mar 13, 2021
@k-takata k-takata deleted the use-japanese-locale branch March 13, 2021 01:55
@k-takata
Copy link
Member Author

知っているかもですが Microsoftのサイトから開発者向けのWindows10の仮想OSイメージ が配布されていて、

この VM の有効期限は 2021 年 3 月 14 日です。

英語版のページを見ると有効期限は5月になっていました。が、ダウンロードのリンク先は英語ページも日本語ページも同じだったので、日本語ページの情報が古いだけのようですね。

@beru
Copy link
Contributor

beru commented Mar 13, 2021

この VM の有効期限は 2021 年 3 月 14 日です。

英語版のページを見ると有効期限は5月になっていました。が、ダウンロードのリンク先は英語ページも日本語ページも同じだったので、日本語ページの情報が古いだけのようですね。

おぉ、本当ですね。自分には英語のページを調べるという発想が浮かびませんでした。

VirtualBox をインストールしてダウンロードしたイメージをインポートしました。
このPRによりEUC-JPのテキストファイルが英語OSで正しく表示されるようになる事が確認出来ました。

@k-takata
Copy link
Member Author

確認ありがとうございます。

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.

EUC-JPのファイルが文字化けする
4 participants