総合掲示板(アップロードファイル合計最大1MB)
新規投稿 ┃ツリー表示 ┃スレッド表示 ┃一覧表示 ┃トピック表示 ┃番号順表示 ┃検索 ┃設定 ┃過去ログ ┃ホーム
64 / 172 ツリー    ←次へ | 前へ→

CrystalDiskInfo 8.7.0 のソースコードについて(バグ?) G神 20/8/16(日) 18:39
Re:CrystalDiskInfo 8.7.0 のソースコードについて(... ひよひよ 20/8/16(日) 18:53
Re:CrystalDiskInfo 8.7.0 のソースコードについて(... G神 20/8/16(日) 19:01
Re:CrystalDiskInfo 8.7.0 のソースコードについて(... G神 20/8/16(日) 19:10
Re:CrystalDiskInfo 8.7.0 のソースコードについて(... ひよひよ 20/8/16(日) 21:43

CrystalDiskInfo 8.7.0 のソースコードに...
 G神  - 20/8/16(日) 18:39 -
CrystalDiskInfo 8.7.0 のソースコードについてお尋ねします。

以下の2つの処理は、いずれも、同じロジック判定を行っているように思えますが、
両者のコードは、丸括弧の括り範囲が微妙に違うため、判定結果は異なります。

AND/OR条件から判断して、CDiskInfoDlg::GetDiskStatusReason側のコードが
間違っているように思います。

いかがでしょうか? 

// AtaSmart.cpp ... CAtaSmart::CheckDiskStatus関数内(9368行〜9377行)
--------------------------------------------------------------------
else
if((vars[i].Attribute[j].Id == 0xE8 && (vars[i].DiskVendorId == SSD_VENDOR_PLEXTOR || vars[i].DiskVendorId == SSD_VENDOR_SANDISK))
|| (vars[i].Attribute[j].Id == 0xBB && vars[i].DiskVendorId == SSD_VENDOR_MTRON)
|| (vars[i].Attribute[j].Id == 0xB1 && vars[i].DiskVendorId == SSD_VENDOR_SAMSUNG)
|| (vars[i].Attribute[j].Id == 0xD1 && vars[i].DiskVendorId == SSD_VENDOR_INDILINX)
|| (vars[i].Attribute[j].Id == 0xE7 && vars[i].DiskVendorId == SSD_VENDOR_SANDFORCE)
|| (vars[i].Attribute[j].Id == 0xAA && vars[i].DiskVendorId == SSD_VENDOR_JMICRON && ! vars[i].IsRawValues8)
|| (vars[i].Attribute[j].Id == 0xCA && vars[i].DiskVendorId == SSD_VENDOR_MICRON)
|| (vars[i].Attribute[j].Id == 0xE9 && (vars[i].DiskVendorId == SSD_VENDOR_INTEL || vars[i].DiskVendorId == SSD_VENDOR_OCZ || vars[i].DiskVendorId == SSD_VENDOR_OCZ_VECTOR))
)

// DiskInfoDlgInit.cpp ... CDiskInfoDlg::GetDiskStatusReason関数内(494行〜504行)
--------------------------------------------------------------------------------
else
if((m_Ata.vars[index].Attribute[j].Id == 0xE8 && (m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_PLEXTOR || m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_SANDISK))
|| (m_Ata.vars[index].Attribute[j].Id == 0xBB && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_MTRON)
|| (m_Ata.vars[index].Attribute[j].Id == 0xB1 && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_SAMSUNG)
|| (m_Ata.vars[index].Attribute[j].Id == 0xD1 && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_INDILINX)
|| (m_Ata.vars[index].Attribute[j].Id == 0xE7 && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_SANDFORCE)
|| (m_Ata.vars[index].Attribute[j].Id == 0xAA && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_JMICRON && ! m_Ata.vars[index].IsRawValues8
|| (m_Ata.vars[index].Attribute[j].Id == 0xCA && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_MICRON)
|| (m_Ata.vars[index].Attribute[j].Id == 0xE9 && (m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_INTEL || m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_OCZ || m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_OCZ_VECTOR))
)
)

引用なし

パスワード


・ツリー全体表示

Re:CrystalDiskInfo 8.7.0 のソースコード...
 ひよひよ  - 20/8/16(日) 18:53 -
▼G神さん:
>CrystalDiskInfo 8.7.0 のソースコードについてお尋ねします。
>
>以下の2つの処理は、いずれも、同じロジック判定を行っているように思えますが、
>両者のコードは、丸括弧の括り範囲が微妙に違うため、判定結果は異なります。
>
>AND/OR条件から判断して、CDiskInfoDlg::GetDiskStatusReason側のコードが
>間違っているように思います。
>
>いかがでしょうか? 
>
>// AtaSmart.cpp ... CAtaSmart::CheckDiskStatus関数内(9368行〜9377行)
>--------------------------------------------------------------------
> else
> if((vars[i].Attribute[j].Id == 0xE8 && (vars[i].DiskVendorId == SSD_VENDOR_PLEXTOR || vars[i].DiskVendorId == SSD_VENDOR_SANDISK))
> || (vars[i].Attribute[j].Id == 0xBB && vars[i].DiskVendorId == SSD_VENDOR_MTRON)
> || (vars[i].Attribute[j].Id == 0xB1 && vars[i].DiskVendorId == SSD_VENDOR_SAMSUNG)
> || (vars[i].Attribute[j].Id == 0xD1 && vars[i].DiskVendorId == SSD_VENDOR_INDILINX)
> || (vars[i].Attribute[j].Id == 0xE7 && vars[i].DiskVendorId == SSD_VENDOR_SANDFORCE)
> || (vars[i].Attribute[j].Id == 0xAA && vars[i].DiskVendorId == SSD_VENDOR_JMICRON && ! vars[i].IsRawValues8)
> || (vars[i].Attribute[j].Id == 0xCA && vars[i].DiskVendorId == SSD_VENDOR_MICRON)
> || (vars[i].Attribute[j].Id == 0xE9 && (vars[i].DiskVendorId == SSD_VENDOR_INTEL || vars[i].DiskVendorId == SSD_VENDOR_OCZ || vars[i].DiskVendorId == SSD_VENDOR_OCZ_VECTOR))
> )
>
>// DiskInfoDlgInit.cpp ... CDiskInfoDlg::GetDiskStatusReason関数内(494行〜504行)
>--------------------------------------------------------------------------------
> else
> if((m_Ata.vars[index].Attribute[j].Id == 0xE8 && (m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_PLEXTOR || m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_SANDISK))
> || (m_Ata.vars[index].Attribute[j].Id == 0xBB && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_MTRON)
> || (m_Ata.vars[index].Attribute[j].Id == 0xB1 && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_SAMSUNG)
> || (m_Ata.vars[index].Attribute[j].Id == 0xD1 && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_INDILINX)
> || (m_Ata.vars[index].Attribute[j].Id == 0xE7 && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_SANDFORCE)
> || (m_Ata.vars[index].Attribute[j].Id == 0xAA && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_JMICRON && ! m_Ata.vars[index].IsRawValues8
> || (m_Ata.vars[index].Attribute[j].Id == 0xCA && m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_MICRON)
> || (m_Ata.vars[index].Attribute[j].Id == 0xE9 && (m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_INTEL || m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_OCZ || m_Ata.vars[index].DiskVendorId == m_Ata.SSD_VENDOR_OCZ_VECTOR))
> )
> )
ご指摘ありがとうございます!
同じようなコードが複数個所にある時点で設計が悪すぎなのですが、8.8.0 AlphaXで直したつもりでしたが、)の不一致がありました。

Alpha12で直します。

引用なし

パスワード


・ツリー全体表示

Re:CrystalDiskInfo 8.7.0 のソースコード...
 G神  - 20/8/16(日) 19:01 -
ちなみに、参照ポインタを使うと、前者は ...

const ATA_SMART_INFO &info=vars[index];

// 中略

else
if((info.Attribute[j].Id == 0xE8 && (info.DiskVendorId == SSD_VENDOR_PLEXTOR || info.DiskVendorId == SSD_VENDOR_SANDISK))
|| (info.Attribute[j].Id == 0xBB && info.DiskVendorId == SSD_VENDOR_MTRON)
|| (info.Attribute[j].Id == 0xB1 && info.DiskVendorId == SSD_VENDOR_SAMSUNG)
|| (info.Attribute[j].Id == 0xD1 && info.DiskVendorId == SSD_VENDOR_INDILINX)
|| (info.Attribute[j].Id == 0xE7 && info.DiskVendorId == SSD_VENDOR_SANDFORCE)
|| (info.Attribute[j].Id == 0xAA && info.DiskVendorId == SSD_VENDOR_JMICRON && ! info.IsRawValues8)
|| (info.Attribute[j].Id == 0xCA && info.DiskVendorId == SSD_VENDOR_MICRON)
|| (info.Attribute[j].Id == 0xE9 && (info.DiskVendorId == SSD_VENDOR_INTEL || info.DiskVendorId == SSD_VENDOR_OCZ || info.DiskVendorId == SSD_VENDOR_OCZ_VECTOR))
)

後者は ...

const ATA_SMART_INFO &info=m_Ata.vars[index];

// 中略

else
if((info.Attribute[j].Id == 0xE8 && (info.DiskVendorId == m_Ata.SSD_VENDOR_PLEXTOR || info.DiskVendorId == m_Ata.SSD_VENDOR_SANDISK))
|| (info.Attribute[j].Id == 0xBB && info.DiskVendorId == m_Ata.SSD_VENDOR_MTRON)
|| (info.Attribute[j].Id == 0xB1 && info.DiskVendorId == m_Ata.SSD_VENDOR_SAMSUNG)
|| (info.Attribute[j].Id == 0xD1 && info.DiskVendorId == m_Ata.SSD_VENDOR_INDILINX)
|| (info.Attribute[j].Id == 0xE7 && info.DiskVendorId == m_Ata.SSD_VENDOR_SANDFORCE)
|| (info.Attribute[j].Id == 0xAA && info.DiskVendorId == m_Ata.SSD_VENDOR_JMICRON && ! info.IsRawValues8)
|| (info.Attribute[j].Id == 0xCA && info.DiskVendorId == m_Ata.SSD_VENDOR_MICRON)
|| (info.Attribute[j].Id == 0xE9 && (info.DiskVendorId == m_Ata.SSD_VENDOR_INTEL || info.DiskVendorId == m_Ata.SSD_VENDOR_OCZ || info.DiskVendorId == m_Ata.SSD_VENDOR_OCZ_VECTOR))
)

と簡潔に書けます。 また、「m_Ata.SSD_VENDOR_PLEXTOR」等のenum値は、「CAtaSmart::SSD_VENDOR_PLEXTOR」
とも書けますが、そもそもenum定数の定義をCAtaSmartクラス外へ出せば、
クラスの指定は不要で、両者のコードは同じになります。

引用なし

パスワード


・ツリー全体表示

Re:CrystalDiskInfo 8.7.0 のソースコード...
 G神  - 20/8/16(日) 19:10 -
ところで、C++では、クラスと構造体は、メンバのデフォルトの扱いが違うだけで、実質的には同等ですが、
ATA_SMART_INFO をクラスではなく構造体として宣言されているのはなぜでしょうか?

構造体でもメンバ関数は定義できますので、僭越ながら、else if()内の判定ロジックをメンバ関数化すれば、
同じ処理を一か所のコードで管理できますので、直し間違いや直し漏れも減ると思います。

具体的には、例えば...

BOOL ATA_SMART_INFO::IsDiskCondition(index j) const
{
if((Attribute[j].Id == 0xE8 && (DiskVendorId == SSD_VENDOR_PLEXTOR || DiskVendorId == SSD_VENDOR_SANDISK))
|| (Attribute[j].Id == 0xBB && DiskVendorId == SSD_VENDOR_MTRON)
|| (Attribute[j].Id == 0xB1 && DiskVendorId == SSD_VENDOR_SAMSUNG)
|| (Attribute[j].Id == 0xD1 && DiskVendorId == SSD_VENDOR_INDILINX)
|| (Attribute[j].Id == 0xE7 && DiskVendorId == SSD_VENDOR_SANDFORCE)
|| (Attribute[j].Id == 0xAA && DiskVendorId == SSD_VENDOR_JMICRON && ! IsRawValues8)
|| (Attribute[j].Id == 0xCA && DiskVendorId == SSD_VENDOR_MICRON)
|| (Attribute[j].Id == 0xE9 && (DiskVendorId == SSD_VENDOR_INTEL || DiskVendorId == SSD_VENDOR_OCZ || DiskVendorId == SSD_VENDOR_OCZ_VECTOR))
return TRUE;
else
return FALSE;
}

というメンバ関数を定義すれば、前者は ...

const ATA_SMART_INFO &info=vars[index];

// 中略

else if(info.IsDiskCondition(j))

後者は ...

const ATA_SMART_INFO &info=m_Ata.vars[index];

// 中略

else if(info.IsDiskCondition(j))

となり、将来の対応機種を追加する際には、ATA_SMART_INFO::IsDiskCondition()関数
のみ変更すれば、両者に適用されます。

引用なし

パスワード


・ツリー全体表示

Re:CrystalDiskInfo 8.7.0 のソースコード...
 ひよひよ  - 20/8/16(日) 21:43 -
▼G神さん:
>ところで、C++では、クラスと構造体は、メンバのデフォルトの扱いが違うだけで、実質的には同等ですが、
>ATA_SMART_INFO をクラスではなく構造体として宣言されているのはなぜでしょうか?
色々アドバイスありがとうございます。なぜ構造体なのか?
CrystalDiskInfoの開発を開始した時点では(2008年)、SSDがこんなに増えるとは想像できていなかったこと、IDENTIFY_DEVICEデータを規格に従って分解していけば適切な対応ができると考えていたこと、そもそも10年以上続けるようなプロジェクトになるとは考えていなかったこと、その後の拡張をひたすら場当たり的に行ってきたことなどが考えられますが、今となっては理由はわかりません。

この状況で戦い続けるのは非常に厳しいので、Ver.9で0から再構築することを考えております。

引用なし

パスワード


・ツリー全体表示

64 / 172 ツリー    ←次へ | 前へ→
新規投稿 ┃ツリー表示 ┃スレッド表示 ┃一覧表示 ┃トピック表示 ┃番号順表示 ┃検索 ┃設定 ┃過去ログ ┃ホーム
 83605
ページ:  ┃  記事番号:  

C-BOARD Moyuku v1.03b5