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

CRuler::DrawRulerBg において TextOutW を呼び出す際に第5引数の型 (int) にキャストする記述を追加して警告回避 #1559

Closed
wants to merge 1 commit into from

Conversation

beru
Copy link
Contributor

@beru beru commented Feb 28, 2021

PR の目的

x64 ビルドで警告が出る数を減らす為の変更です。

カテゴリ

  • リファクタリング

PR の背景

x64 のビルドで警告が大量に出る事が #430 で話されています。

PR のメリット

sakura プロジェクトの x64 Debug ビルド時の警告の数が 601 から 600 になり、1個減ります。
x64 Release ビルド時の警告の数が 566 から 565 になり、1個減ります。
Win32 Debug と Win32 Release ビルド時の警告の数は 0 のままです。
(変更前のコミットは 32cdaf0

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

キャストの記述の分だけコードの情報量が増える。

仕様・動作説明

呼び出すWindowsAPIの関数 TextOutW の第5引数の型は int で第4引数に渡した文字列バッファの長さを指定するとの事です。

https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-textoutw

長さの定義については下記に記載されていますが、-1 を指定する事で呼び出し先の関数でNULL終端文字列の長さを調べてくれるお手軽仕様です。なので長さのパラメータなのに符号付の型になっているようです。

https://docs.microsoft.com/en-us/windows/win32/gdi/specifying-length-of-text-output-string

という事は wcslen( szColumn ) の記述にキャストを入れるのではなくて -1 に置き換えてしまうのもありかもしれないですね。

PR の影響範囲

今までは暗黙的に int にキャストしていたものを明示的にキャストする記述にしましたが、動作への影響はないと思います。

https://docs.microsoft.com/en-us/cpp/cpp/type-conversions-and-type-safety-modern-cpp?view=msvc-160#narrowing-conversions-coercion

によると

The compiler performs narrowing conversions implicitly, but it warns you about potential data loss. Take these warnings very seriously. If you are certain that no data loss will occur because the values in the larger variable will always fit in the smaller variable, then add an explicit cast so that the compiler will no longer issue a warning.

という事で、今回はキャスト前の値がキャスト先の型の範囲内に収まる事が明確なので明示的なキャストで何の問題もないと考えます。

テスト内容

テスト1

  • タイプ別設定のスクリーンのレイアウトが折り返し桁数 10240、折り返し方法は「折り返さない」でエディタ領域を右端まで横スクロールしてルーラーの10桁おきの数字が正しく描画される事を確認

関連 issue, PR

#430 #1541

参考資料

@beru beru added the refactoring リファクタリング 【ChangeLog除外】 label Feb 28, 2021
@beru
Copy link
Contributor Author

beru commented Feb 28, 2021

テキスト描画で TextOut マクロでWindowsAPIの TextOutW 関数を呼び出す記述が全部で6か所ありますが、全てにおいて第5引数の長さの値を求めるのに wcslen 関数が使われています。int へのキャストが無いので x64 ビルドだと警告になってそうです。文字列リテラルを渡している箇所では直接数字を書いてよい気がするのと、他の個所では -1 を渡してしまうのが良いように思えます。

変更をするなら(問題が無さそうでも)念の為に個別に動作確認を取る必要はあると思います。

@sonarcloud
Copy link

sonarcloud bot commented Feb 28, 2021

@beru
Copy link
Contributor Author

beru commented Feb 28, 2021

キャストを追加するのではなくて #1559 (comment) で書いた対応を取ることにするので close します。

@beru beru closed this Feb 28, 2021
@beru beru deleted the x64_fix_warning_DrawRulerBg branch February 28, 2021 18:47
@AppVeyorBot
Copy link

Build sakura 1.0.3493 failed (commit 0fb5c23bad by @beru)

@beru
Copy link
Contributor Author

beru commented Mar 6, 2021

TextOutW 関数の第5引数は -1 に対応していない事を確認しました。
このPRを作成したことを忘れていて #1569 を作成しました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring リファクタリング 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants