総合掲示板（アップロードファイル合計最大1MB）
 |
 |
今回の指摘内容は、バグではないです。(w G神 20/9/22(火) 20:56 |
|
 |
Re:今回の指摘内容は、バグではないです。(w G神 20/9/22(火) 21:01 |
|
  |
Re:今回の指摘内容は、バグではないです。(w ひよひよ 20/9/23(水) 0:50 |
|
|
CDI8.9.0で対処が入ったのかな？ 金華山の仙人 20/12/16(水) 20:48 |
|
|
凄い手間暇を掛けたアドバイス、ご苦労様です。 金華山の仙人 20/9/24(木) 11:25 |
|
G神 - 20/9/22(火) 20:56 - |
現在公開されている最新の「8.8.7」で確認して、幾つか気がついた点があったのでまとめてご報告します。
まず、メインダイアログ(CDiskInfoDlg クラス)の以下の関数は、ボタン押下(BN_CLICKED)メッセージハンドラと思われますが、現在はコメントで書いた関数へ変更されており、中身は同じ処理ですが、こちらは現在使われていません。
宣言: DiskInfoDlg.h ............. 346行〜 356行
実装: DiskInfoDlgUpdate.cpp ..... 2214行〜2225行
void OnDisk0(); // Replaced! → afx_msg void OnBnClickedButtonDisk0();
void OnDisk1(); // Replaced! → afx_msg void OnBnClickedButtonDisk1();
void OnDisk2(); // Replaced! → afx_msg void OnBnClickedButtonDisk2();
void OnDisk3(); // Replaced! → afx_msg void OnBnClickedButtonDisk3();
void OnDisk4(); // Replaced! → afx_msg void OnBnClickedButtonDisk4();
void OnDisk5(); // Replaced! → afx_msg void OnBnClickedButtonDisk5();
void OnDisk6(); // Replaced! → afx_msg void OnBnClickedButtonDisk6();
void OnDisk7(); // Replaced! → afx_msg void OnBnClickedButtonDisk7();
void OnPreDisk(); // Replaced! → afx_msg void OnBnClickedButtonPreDisk();
void OnNextDisk(); // Replaced! → afx_msg void OnBnClickedButtonNextDisk();
void OnDiskStatus(); // Replaced! → afx_msg void OnBnClickedButtonHealthStatus();
※ CDiskInfoDlg::OnDiskStatus()は、CDiskInfoDlg::OnHealthStatus()を呼んでいますが、メッセージマップには、OnDiskStatus()と関連付けるエントリーがありません。
また、呼び出されているCDiskInfoDlg::OnHealthStatus()ですが、この関数の中身は、CDiskInfoDlg::OnBnClickedButtonHealthStatus()とまったく同じです。
前者の OnHealthStatus() は メニュー用のID = ID_HEALTH_STATUS ........................... 値:32982
後者の OnBnClickedButtonHealthStatus() は PUSHボタンのID = IDC_BUTTON_HEALTH_STATUS .... 値:1047
のメッセージハンドラとして登録されています。
ON_COMMAND(ID_HEALTH_STATUS, &CDiskInfoDlg::OnHealthStatus)
ON_BN_CLICKED(IDC_BUTTON_HEALTH_STATUS, &CDiskInfoDlg::OnBnClickedButtonHealthStatus)
(1) afx_msg void CDiskInfoDlg::OnHealthStatus()
宣言: DiskInfoDlg.h ............. 539行
実装: DiskInfoDlgMenu.cpp ....... 76行〜 84行
(2) afx_msg void CDiskInfoDlg::OnBnClickedButtonHealthStatus();
宣言: DiskInfoDlg.h ............. 581行
実装: DiskInfoDlg.cpp ........... 2395行〜2403行
また、以下の2つの関数は、関数名と返す値の型(int/DWORD)こそ違いますが、どちらも中身はメンバ変数「m_SelectDisk」の値を返していて同じ処理です。
後者はプログラム中で一切使われていません。
(1) ....... CDiskInfoDlg::GetSelectedDrive();
宣言: DiskInfoDlg.h ............. 384行
実装: DiskInfoDlgUpdate.cpp ..... 2227行〜2230行
(2) ....... DWORD CDiskInfoDlg::GetSelectDisk(); ← 使われていない
宣言: DiskInfoDlg.h ............. 103行
実装: DiskInfoDlg.cpp ........... 2303行〜2306行
ちなみに、クラスオブジェクトの内容を変更しない関数は、constを付ける習慣を付けた方が良いと思います。 参照やポインタで渡す引数についても、関数内で対象の引数の中身(文字列やオブジェクト)を変更しない場合についても同様です。
int CDiskInfoDlg::GetSelectedDrive();
↓
int CDiskInfoDlg::GetSelectedDrive() const;
文字数制限の為、投稿を分けます。
まず、メインダイアログ(CDiskInfoDlg クラス)の以下の関数は、ボタン押下(BN_CLICKED)メッセージハンドラと思われますが、現在はコメントで書いた関数へ変更されており、中身は同じ処理ですが、こちらは現在使われていません。
宣言: DiskInfoDlg.h ............. 346行〜 356行
実装: DiskInfoDlgUpdate.cpp ..... 2214行〜2225行
void OnDisk0(); // Replaced! → afx_msg void OnBnClickedButtonDisk0();
void OnDisk1(); // Replaced! → afx_msg void OnBnClickedButtonDisk1();
void OnDisk2(); // Replaced! → afx_msg void OnBnClickedButtonDisk2();
void OnDisk3(); // Replaced! → afx_msg void OnBnClickedButtonDisk3();
void OnDisk4(); // Replaced! → afx_msg void OnBnClickedButtonDisk4();
void OnDisk5(); // Replaced! → afx_msg void OnBnClickedButtonDisk5();
void OnDisk6(); // Replaced! → afx_msg void OnBnClickedButtonDisk6();
void OnDisk7(); // Replaced! → afx_msg void OnBnClickedButtonDisk7();
void OnPreDisk(); // Replaced! → afx_msg void OnBnClickedButtonPreDisk();
void OnNextDisk(); // Replaced! → afx_msg void OnBnClickedButtonNextDisk();
void OnDiskStatus(); // Replaced! → afx_msg void OnBnClickedButtonHealthStatus();
※ CDiskInfoDlg::OnDiskStatus()は、CDiskInfoDlg::OnHealthStatus()を呼んでいますが、メッセージマップには、OnDiskStatus()と関連付けるエントリーがありません。
また、呼び出されているCDiskInfoDlg::OnHealthStatus()ですが、この関数の中身は、CDiskInfoDlg::OnBnClickedButtonHealthStatus()とまったく同じです。
前者の OnHealthStatus() は メニュー用のID = ID_HEALTH_STATUS ........................... 値:32982
後者の OnBnClickedButtonHealthStatus() は PUSHボタンのID = IDC_BUTTON_HEALTH_STATUS .... 値:1047
のメッセージハンドラとして登録されています。
ON_COMMAND(ID_HEALTH_STATUS, &CDiskInfoDlg::OnHealthStatus)
ON_BN_CLICKED(IDC_BUTTON_HEALTH_STATUS, &CDiskInfoDlg::OnBnClickedButtonHealthStatus)
(1) afx_msg void CDiskInfoDlg::OnHealthStatus()
宣言: DiskInfoDlg.h ............. 539行
実装: DiskInfoDlgMenu.cpp ....... 76行〜 84行
(2) afx_msg void CDiskInfoDlg::OnBnClickedButtonHealthStatus();
宣言: DiskInfoDlg.h ............. 581行
実装: DiskInfoDlg.cpp ........... 2395行〜2403行
また、以下の2つの関数は、関数名と返す値の型(int/DWORD)こそ違いますが、どちらも中身はメンバ変数「m_SelectDisk」の値を返していて同じ処理です。
後者はプログラム中で一切使われていません。
(1) ....... CDiskInfoDlg::GetSelectedDrive();
宣言: DiskInfoDlg.h ............. 384行
実装: DiskInfoDlgUpdate.cpp ..... 2227行〜2230行
(2) ....... DWORD CDiskInfoDlg::GetSelectDisk(); ← 使われていない
宣言: DiskInfoDlg.h ............. 103行
実装: DiskInfoDlg.cpp ........... 2303行〜2306行
ちなみに、クラスオブジェクトの内容を変更しない関数は、constを付ける習慣を付けた方が良いと思います。 参照やポインタで渡す引数についても、関数内で対象の引数の中身(文字列やオブジェクト)を変更しない場合についても同様です。
int CDiskInfoDlg::GetSelectedDrive();
↓
int CDiskInfoDlg::GetSelectedDrive() const;
文字数制限の為、投稿を分けます。
|
G神 - 20/9/22(火) 21:01 - |
以下、続きです。
現在、ユーザー定義メッセージが、
#define WM_THEME_ID (WM_APP + 0x0100) // 0x8000 + 0x0100 = 33024〜(33024+テーマ数-1)
#define WM_LANGUAGE_ID (WM_APP + 0x0200) // 0x8000 + 0x0200 = 33280〜(33280+言語数-1)
となっていますが、「resource.h」に登録されたメニューIDを見ると、この先テーマ数が増えた際、WM_LANGUAGE_IDの範囲と被るより前に、既存のメニューIDと被って思わずバグりそうなので、思い切って、WM_APP+0x200/WM_APP+0x300とかに直した方がよいと思います。
また、以下のID並びは「33127」が飛んでいます。
#define ID_AUTO_DETECTION_05_SEC 33125
#define ID_AUTO_DETECTION_10_SEC 33126
#define ID_AUTO_DETECTION_20_SEC 33128
#define ID_AUTO_DETECTION_30_SEC 33129
#define ID_AUTO_DETECTION_DISABLE 33130
CDiskInfoDlg::CheckRadioAutoDetection()内の処理、
pMenu->CheckMenuRadioItem(ID_AUTO_DETECTION_05_SEC,ID_AUTO_DETECTION_DISABLE,id,MF_BYCOMMAND);
で都合が悪いかもしれません。
似た処理はメニューID値(番号)を連続する値に割り当ててやれば、「ON_COMMAND_RANGE」マクロを使ってメッセージハンドラを共通化できますし、設定項目の追加や変更も容易です。
[.h ファイル]
class CDiskInfoDlg : public CMainDialogFx
{
// 中略
private:
static CONST INT s_intAutoDetectSetting[];
static CONST INT s_intAutoDetectChoice;
// 中略
protected:
afx_msg void OnAutoDetection(UINT nCommandID);
// 中略
};
[.cpp ファイル]
CONST INT CDiskInfoDlg::s_intAutoDetectSetting[]=
{5 /* [0]:ID_AUTO_DETECTION_05_SEC */
,10 /* [1]:ID_AUTO_DETECTION_10_SEC */
,20 /* [2]:ID_AUTO_DETECTION_20_SEC */
,30 /* [3]:ID_AUTO_DETECTION_30_SEC */
,0 /* [4]:ID_AUTO_DETECTION_DISABLE */ };
CONST INT CDiskInfoDlg::s_intAutoDetectChoice = _countof(s_intAutoDetectSetting);
ON_COMMAND_RANGE(ID_AUTO_DETECTION_05_SEC, ID_AUTO_DETECTION_DISABLE, &CDiskInfoDlg::OnAutoDetection)
void CDiskInfoDlg::OnAutoDetection(UINT nCommandID)
{
if(ID_AUTO_DETECTION_05_SEC<=nCommandID && nCommandID<=ID_AUTO_DETECTION_DISABLE)
{
int nIndex;
nIndex=nCommandID-ID_AUTO_DETECTION_05_SEC;
if(0<=nIndex && nIndex<s_intAutoDetectChoice)
CheckRadioWaitTime(nCommandID, s_intAutoDetectSetting[nIndex]);
else
ASSERT(FALSE); /* Short table entry or command rang is not match. */
}
else
{
TRACE(_T("\r\nERROR : Unknown ID=0x%08X, in %s"),nCommandID,_T(__FUNCTION__));
ASSERT(FALSE);
}
}
なお、念のため CDiskInfoDlg のコンストラクタ内にでも、以下のようなASSERTマクロを埋めておけば、
ASSERT( ID_AUTO_DETECTION_10_SEC == ID_AUTO_DETECTION_05_SEC + 1 );
ASSERT( ID_AUTO_DETECTION_20_SEC == ID_AUTO_DETECTION_10_SEC + 1 );
ASSERT( ID_AUTO_DETECTION_30_SEC == ID_AUTO_DETECTION_20_SEC + 1 );
ASSERT( ID_AUTO_DETECTION_DISABLE == ID_AUTO_DETECTION_30_SEC + 1 );
ASSERT((ID_AUTO_DETECTION_DISABLE - ID_AUTO_DETECTION_05_SEC + 1) == _countof(s_intAutoDetectSetting));
[1] IDがちゃんと並び順になっているかどうか
[2] ID範囲の個数とテーブルの要素数が一致しているか
を、Debugビルドの実行時にチェックできます。
※ ASSERTマクロは、Releaseビルド時のコード生成に影響を与えませんが、
関数の戻り値をチェックするのに、関数呼び出しをASSERTマクロで囲むと、
Releaseビルドで、関数の呼出(実行)も行われなくなるので、VERIFY()
マクロを使うか、戻り値を一旦変数に格納して、ASSERTマクロで変数の
値をチェックするようにしてください。
それから、DiskInfoDlgUpdate.cppの 1605行目 ...
cstr = i18n(_T("Menu"), _T("ALERM_HISTORY"));
ですが、
【誤】 ......「ALERM_HISTORY」
【正】 ......「ALARM_HISTORY」
だと思います。
現在「ID_ALARM_HISTORY」メニューは使われていないので実害はありませんが、綴りが違うので、言語ファイル(.lang)からの読み込みに失敗して i18n() から
空文字列("")が返されるとともに、「ID_ALARM_HISTORY」メニューは存在しないので、
menu->ModifyMenu(ID_ALARM_HISTORY, MF_STRING, ID_ALARM_HISTORY, cstr);
の呼び出しが失敗(戻り値がFALSE)になります。
関数の戻り値をチェックするように、ModifyMenuの呼び出しをVERIFY()マクロで囲んでやると、Debug/Releaseビルドとも、当該行でエラーが発生します。
最後に、これはご本人次第になりますが、逐次対応するのは大変だと思うので、致命的な不具合でもない限り、更新のリリースは月1回とか、2ヶ月に1回とかに限定した方がよくないですか？
以上、ご報告とご提案まで。
現在、ユーザー定義メッセージが、
#define WM_THEME_ID (WM_APP + 0x0100) // 0x8000 + 0x0100 = 33024〜(33024+テーマ数-1)
#define WM_LANGUAGE_ID (WM_APP + 0x0200) // 0x8000 + 0x0200 = 33280〜(33280+言語数-1)
となっていますが、「resource.h」に登録されたメニューIDを見ると、この先テーマ数が増えた際、WM_LANGUAGE_IDの範囲と被るより前に、既存のメニューIDと被って思わずバグりそうなので、思い切って、WM_APP+0x200/WM_APP+0x300とかに直した方がよいと思います。
また、以下のID並びは「33127」が飛んでいます。
#define ID_AUTO_DETECTION_05_SEC 33125
#define ID_AUTO_DETECTION_10_SEC 33126
#define ID_AUTO_DETECTION_20_SEC 33128
#define ID_AUTO_DETECTION_30_SEC 33129
#define ID_AUTO_DETECTION_DISABLE 33130
CDiskInfoDlg::CheckRadioAutoDetection()内の処理、
pMenu->CheckMenuRadioItem(ID_AUTO_DETECTION_05_SEC,ID_AUTO_DETECTION_DISABLE,id,MF_BYCOMMAND);
で都合が悪いかもしれません。
似た処理はメニューID値(番号)を連続する値に割り当ててやれば、「ON_COMMAND_RANGE」マクロを使ってメッセージハンドラを共通化できますし、設定項目の追加や変更も容易です。
[.h ファイル]
class CDiskInfoDlg : public CMainDialogFx
{
// 中略
private:
static CONST INT s_intAutoDetectSetting[];
static CONST INT s_intAutoDetectChoice;
// 中略
protected:
afx_msg void OnAutoDetection(UINT nCommandID);
// 中略
};
[.cpp ファイル]
CONST INT CDiskInfoDlg::s_intAutoDetectSetting[]=
{5 /* [0]:ID_AUTO_DETECTION_05_SEC */
,10 /* [1]:ID_AUTO_DETECTION_10_SEC */
,20 /* [2]:ID_AUTO_DETECTION_20_SEC */
,30 /* [3]:ID_AUTO_DETECTION_30_SEC */
,0 /* [4]:ID_AUTO_DETECTION_DISABLE */ };
CONST INT CDiskInfoDlg::s_intAutoDetectChoice = _countof(s_intAutoDetectSetting);
ON_COMMAND_RANGE(ID_AUTO_DETECTION_05_SEC, ID_AUTO_DETECTION_DISABLE, &CDiskInfoDlg::OnAutoDetection)
void CDiskInfoDlg::OnAutoDetection(UINT nCommandID)
{
if(ID_AUTO_DETECTION_05_SEC<=nCommandID && nCommandID<=ID_AUTO_DETECTION_DISABLE)
{
int nIndex;
nIndex=nCommandID-ID_AUTO_DETECTION_05_SEC;
if(0<=nIndex && nIndex<s_intAutoDetectChoice)
CheckRadioWaitTime(nCommandID, s_intAutoDetectSetting[nIndex]);
else
ASSERT(FALSE); /* Short table entry or command rang is not match. */
}
else
{
TRACE(_T("\r\nERROR : Unknown ID=0x%08X, in %s"),nCommandID,_T(__FUNCTION__));
ASSERT(FALSE);
}
}
なお、念のため CDiskInfoDlg のコンストラクタ内にでも、以下のようなASSERTマクロを埋めておけば、
ASSERT( ID_AUTO_DETECTION_10_SEC == ID_AUTO_DETECTION_05_SEC + 1 );
ASSERT( ID_AUTO_DETECTION_20_SEC == ID_AUTO_DETECTION_10_SEC + 1 );
ASSERT( ID_AUTO_DETECTION_30_SEC == ID_AUTO_DETECTION_20_SEC + 1 );
ASSERT( ID_AUTO_DETECTION_DISABLE == ID_AUTO_DETECTION_30_SEC + 1 );
ASSERT((ID_AUTO_DETECTION_DISABLE - ID_AUTO_DETECTION_05_SEC + 1) == _countof(s_intAutoDetectSetting));
[1] IDがちゃんと並び順になっているかどうか
[2] ID範囲の個数とテーブルの要素数が一致しているか
を、Debugビルドの実行時にチェックできます。
※ ASSERTマクロは、Releaseビルド時のコード生成に影響を与えませんが、
関数の戻り値をチェックするのに、関数呼び出しをASSERTマクロで囲むと、
Releaseビルドで、関数の呼出(実行)も行われなくなるので、VERIFY()
マクロを使うか、戻り値を一旦変数に格納して、ASSERTマクロで変数の
値をチェックするようにしてください。
それから、DiskInfoDlgUpdate.cppの 1605行目 ...
cstr = i18n(_T("Menu"), _T("ALERM_HISTORY"));
ですが、
【誤】 ......「ALERM_HISTORY」
【正】 ......「ALARM_HISTORY」
だと思います。
現在「ID_ALARM_HISTORY」メニューは使われていないので実害はありませんが、綴りが違うので、言語ファイル(.lang)からの読み込みに失敗して i18n() から
空文字列("")が返されるとともに、「ID_ALARM_HISTORY」メニューは存在しないので、
menu->ModifyMenu(ID_ALARM_HISTORY, MF_STRING, ID_ALARM_HISTORY, cstr);
の呼び出しが失敗(戻り値がFALSE)になります。
関数の戻り値をチェックするように、ModifyMenuの呼び出しをVERIFY()マクロで囲んでやると、Debug/Releaseビルドとも、当該行でエラーが発生します。
最後に、これはご本人次第になりますが、逐次対応するのは大変だと思うので、致命的な不具合でもない限り、更新のリリースは月1回とか、2ヶ月に1回とかに限定した方がよくないですか？
以上、ご報告とご提案まで。
|
ひよひよ
- 20/9/23(水) 0:50 -
|
▼G神さん：
ご指摘ありがとうございます。
影響範囲が小さくすぐに直せる部分は 8.x で修正しつつ、根本的には、9.xで改善を図りたいと思います。
ALARM_HISTORY機能を実装しようとちょっとやり始めたものの見送ったため、その残骸が残っていました。スペリングミスは想定外でしたが。
ご指摘ありがとうございます。
影響範囲が小さくすぐに直せる部分は 8.x で修正しつつ、根本的には、9.xで改善を図りたいと思います。
ALARM_HISTORY機能を実装しようとちょっとやり始めたものの見送ったため、その残骸が残っていました。スペリングミスは想定外でしたが。
|
金華山の仙人 - 20/9/24(木) 11:25 - |
▼G神さん：
自分もプログラム開発現場を離れた後で、離れた先の業務改善（手抜き？）や趣味の為等で、自作のHTMLやマクロの類を3〜4種類作成しましたが、ソースのチェックをしてくれる仲間が居ない場合に、結構苦労しました。
此処まで詳しくソースチェックしてくれるのは、大変貴重で有難い事だと思います。
（まあチェックされた方は、ちょっと複雑な気分になったりしますけどね。^^;）ｲｯｼｭﾉ､ﾊｼﾞｻﾗｼﾆﾅﾙﾉﾃﾞ
自分もプログラム開発現場を離れた後で、離れた先の業務改善（手抜き？）や趣味の為等で、自作のHTMLやマクロの類を3〜4種類作成しましたが、ソースのチェックをしてくれる仲間が居ない場合に、結構苦労しました。
此処まで詳しくソースチェックしてくれるのは、大変貴重で有難い事だと思います。
（まあチェックされた方は、ちょっと複雑な気分になったりしますけどね。^^;）ｲｯｼｭﾉ､ﾊｼﾞｻﾗｼﾆﾅﾙﾉﾃﾞ
|
金華山の仙人 - 20/12/16(水) 20:48 - |
▼ひよひよさん：
>▼G神さん：
>ご指摘ありがとうございます。
>
>影響範囲が小さくすぐに直せる部分は 8.x で修正しつつ、根本的には、9.xで改善を図りたいと思います。
>
>ALARM_HISTORY機能を実装しようとちょっとやり始めたものの見送ったため、その残骸が残っていました。スペリングミスは想定外でしたが。
CDI8.9.0で、JMicronなSSDの画像セーブNG（at W10-1909）が解決したのは、これのお陰でしょうかね？
『CDI8.9.0のJMicronなCDI画像ｾｰﾌﾞ機能OK(at 1909)。』
https://crystalmark.info/board/c-board.cgi?cmd=one;no=2106;id=#2106
そうならG神さんの貢献は、非常に大きいです。感謝感謝です。(^_^)/
>▼G神さん：
>ご指摘ありがとうございます。
>
>影響範囲が小さくすぐに直せる部分は 8.x で修正しつつ、根本的には、9.xで改善を図りたいと思います。
>
>ALARM_HISTORY機能を実装しようとちょっとやり始めたものの見送ったため、その残骸が残っていました。スペリングミスは想定外でしたが。
CDI8.9.0で、JMicronなSSDの画像セーブNG（at W10-1909）が解決したのは、これのお陰でしょうかね？
『CDI8.9.0のJMicronなCDI画像ｾｰﾌﾞ機能OK(at 1909)。』
https://crystalmark.info/board/c-board.cgi?cmd=one;no=2106;id=#2106
そうならG神さんの貢献は、非常に大きいです。感謝感謝です。(^_^)/