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

CPPA::stdErrorにテストを導入したい #1811

Closed
1 of 2 tasks
berryzplus opened this issue Feb 27, 2022 · 4 comments
Closed
1 of 2 tasks

CPPA::stdErrorにテストを導入したい #1811

berryzplus opened this issue Feb 27, 2022 · 4 comments

Comments

@berryzplus
Copy link
Contributor

berryzplus commented Feb 27, 2022

(必須) やりたいこと(=実現したいこと)

#1722 で報告された CPPA::stdError のバグを修正したいです。

修正にあたっての課題事項がたくさんあるので issue 化します。

修正にあたっての課題

  • 該当コードは「外部ライブラリ(PPA.DLL)」に依存しているため、アプリの実動作でバグを確認することは困難または不可能です。
    👉つまり、不具合の再現や修正の効果確認を行う方法がありません。
  • 該当コードは「外部ライブラリ(PPA.DLL)」に依存しているため、文字列を「SJISエンコードのバイトシーケンス」で扱う必要があります。現状のサクラエディタは「文字列とはワイド文字列のことである」的な方向に進んでいて、std::string を使うコードが書きづらくなっています。
    👉つまり、std::string を扱えるように独自定義関数を機能拡張する必要があります。

(省略可) 解決手段の提案

  • 該当コード(CPPA::stdError)の単体テストを用意する。
    アプリ実動作での確認が不可能なので、該当処理を単体で動かせるようにします。
    実現すれば、不具合の再現や修正の効果確認を出来るようになります。
  • strprintf を拡張してstd::string を扱えるようにする。(strprintfを改良してナロー文字も扱えるようにする #1810)
    固定長の char[] を多用する現状のコードに手を入れるには、CとC++をシームレスに繋ぐ独自関数が必要です。
    実現すれば、元々の処理をあまり変えずに修正できるようになります。
@berryzplus berryzplus changed the title CPPA::stdErrorのバグを修正したい CPPA::stdErrorにテストを導入したい Mar 2, 2022
@berryzplus
Copy link
Contributor Author

目的が変わったのでタイトル変えておきます。

報告されていた不具合は、バグじゃなさそう、ということが分かったため。
(ただし、IsBadStringPtrAの利用は「知る人ぞ知る」系の「最も危険な種類」のバグです。実害はないと思いますが。)

@berryzplus
Copy link
Contributor Author

ぶっちゃけると、そもそも関数名からして「誤っている」んですよね。

誤) CPPA::stdError
正) CPPA::ErrorProc

PPAが要求するのは ErrorProc であり stdError は呼出側で勝手に付けた名前です。
privateな関数名なので好きな名前を付けていいんですが、
stdErrorは誤解を招きやすいのでやめたほうが良いと思います。

  • std はCプログラマ的に「標準の~」を意味する接頭辞です。
    定義しているものは明確な「非標準関数」(ユーザー定義関数ともいう)なので、不適切です。
  • stderrは「標準エラー出力(ストリーム)」を指します。
    stdError をユーザー定義関数の名前とするのは誤解を招きやすく、不適切だと思います。
    PPAから要求されている ErrorProc を作るならともかく、関数名から処理内容を推測できないのもイタいと思います。

名前付けの「センスがない」のは「罪じゃない」です。
気付いた人が突っ込んで、直していけばいいだけのことだと考えています。

@berryzplus
Copy link
Contributor Author

berryzplus commented Mar 19, 2022

CPPA::stdErrorはプライベートなグローバル変数CPPA::m_CurInstanceに依存します。

void __stdcall CPPA::stdError( int Err_CD, const char* Err_Mes )
{
if( false != m_CurInstance->m_bError ){
return;
}
m_CurInstance->m_bError = true; // 関数内で関数を呼ぶ場合等、2回表示されるのを防ぐ

CPPA::m_CurInstanceはCPPAの実行コンテキストを保持する構造体です。
プライベートなグローバル変数ですので、クラス外から値を変更することはできません。
CPPA::m_CurInstanceの初期値はnullptrなので、上記コードをテストコードから呼び出すとAV例外が発生します。

テストしたいコードは339行目以降なので、CPPA::stdErrorをテストするにはCPPA::m_CurInstanceに有効なPPAコンテキストを設定するために仕組みが必須になります。

@berryzplus
Copy link
Contributor Author

別な対応方法を検討しているので取り下げます。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant