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

デバッグ実行時に外部コマンド実行ダイアログを開くとwarning_point関数で止まる問題を修正する #1440

Merged

Conversation

suconbu
Copy link
Member

@suconbu suconbu commented Oct 26, 2020

PR の目的

外部コマンド実行ダイアログを開く時、StaticVectorの有効範囲外参照 (要素数 0 の時に 0 番目を参照) によりwarning_point関数でブレークしてしまう点を解消します。
この現象は「名前」の履歴がない状態において 100% 発生します。

ただ、有効範囲外ではありますが有効なメモリ領域が参照され空文字列が取得されているために、実動作上の問題にはなっていないようです。

#1421 の確認中に発見しました。

カテゴリ

  • 不具合修正

PR の背景

PR のメリット

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

仕様・動作説明

  • 履歴の数が 0 の場合にはコンボボックスへのデータ設定をスキップします。(コマンド履歴、カレントディレクトリについて)

テスト内容

  1. VisualStudio でデバッグ実行を開始する。
  2. 履歴の管理ダイアログを開き、「コマンド」「カレントディレクトリ」の履歴をそれぞれすべて削除する。
  3. 外部コマンド実行ダイアログを開く。
  • 確認: warning_point関数でブレークしないこと。

PR の影響範囲

関連 issue, PR

参考資料

@suconbu suconbu added the 🐛bug🦋 ■バグ修正(Something isn't working) label Oct 26, 2020
@AppVeyorBot
Copy link

Build sakura 1.0.3192 completed (commit 0e63b6c7c4 by @suconbu)

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

変更目的は理解しました。やったほうがいい修正だと思います。

コンボを一旦クリアする処理をif文の外に出す件と
const付ける件は修正してから入れたいです。

sakura_core/dlg/CDlgExec.cpp Show resolved Hide resolved
sakura_core/dlg/CDlgExec.cpp Outdated Show resolved Hide resolved
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();
Copy link
Contributor

Choose a reason for hiding this comment

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

この変数宣言にツッコミ2件あります。

  • 変更不可にすると困る場合を除いて、constを付けるべきだと思います。
  • size() の戻り値を受けるのに型が intなことに違和感があります。
    size()の戻り型って普通は std::size_t ですよね? 😃

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

  • StaticVector::size() が int を返すのでした。

まー、その事実を踏まえて、コメントだけで「修正したい」と言ってないわけですが...(笑)

Copy link
Member Author

Choose a reason for hiding this comment

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

指摘と誤解してしまいました😅

@suconbu
Copy link
Member Author

suconbu commented Oct 27, 2020

コンボを一旦クリアする処理をif文の外に出す件と
const付ける件は修正してから入れたいです。

指摘頂いた点が修正できましたのでご確認お願いします。

@AppVeyorBot
Copy link

Build sakura 1.0.3198 completed (commit aa539a46f9 by @suconbu)

@suconbu
Copy link
Member Author

suconbu commented Oct 28, 2020

レビューありがとうございました。マージします。

@suconbu suconbu merged commit b2239d5 into sakura-editor:master Oct 28, 2020
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.

3 participants