デバッグ実行時に外部コマンド実行ダイアログを開くとwarning_point関数で止まる問題を修正する #1440
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ Build sakura 1.0.3192 completed (commit 0e63b6c7c4 by @suconbu) |
berryzplus
requested changes
Oct 27, 2020
sakura_core/dlg/CDlgExec.cpp
Outdated
| int nSize = m_pShareData->m_sHistory.m_aCommands.size(); | ||
| for( i = 0; i < nSize; ++i ){ | ||
| Combo_AddString( hwndCombo, m_pShareData->m_sHistory.m_aCommands[i] ); | ||
| int nCommandsCount = m_pShareData->m_sHistory.m_aCommands.size(); |
There was a problem hiding this comment.
この変数宣言にツッコミ２件あります。
- 変更不可にすると困る場合を除いて、constを付けるべきだと思います。
size()の戻り値を受けるのに型がintなことに違和感があります。
size()の戻り型って普通は std::size_t ですよね？ 😃
There was a problem hiding this comment.
- const をつけます。
- StaticVector::size() が int を返すのでした。
sakura/sakura_core/util/StaticType.h
Line 41 in d0a92d0
There was a problem hiding this comment.
- StaticVector::size() が int を返すのでした。
まー、その事実を踏まえて、コメントだけで「修正したい」と言ってないわけですが...(笑)
指摘頂いた点が修正できましたのでご確認お願いします。 |
|
✅ Build sakura 1.0.3198 completed (commit aa539a46f9 by @suconbu) |
ghost
mentioned this pull request
berryzplus
approved these changes
Oct 28, 2020
|
レビューありがとうございました。マージします。 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR の目的
外部コマンド実行ダイアログを開く時、
StaticVectorの有効範囲外参照 (要素数 0 の時に 0 番目を参照) によりwarning_point関数でブレークしてしまう点を解消します。この現象は「名前」の履歴がない状態において 100% 発生します。
ただ、有効範囲外ではありますが有効なメモリ領域が参照され空文字列が取得されているために、実動作上の問題にはなっていないようです。
※ #1421 の確認中に発見しました。
カテゴリ
PR の背景
PR のメリット
PR のデメリット (トレードオフとかあれば)
仕様・動作説明
テスト内容
warning_point関数でブレークしないこと。PR の影響範囲
関連 issue, PR
参考資料