Ｐａｇｅ 337 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 通常モードに戻る ┃ ＩＮＤＥＸ ┃ ≪前へ │ 次へ≫ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ▼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)) ) ) |
| ▼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で直します。 |
| ちなみに、参照ポインタを使うと、前者は ... 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クラス外へ出せば、 クラスの指定は不要で、両者のコードは同じになります。 |
| ところで、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()関数 のみ変更すれば、両者に適用されます。 |
| ▼G神さん： >ところで、C++では、クラスと構造体は、メンバのデフォルトの扱いが違うだけで、実質的には同等ですが、 >ATA_SMART_INFO をクラスではなく構造体として宣言されているのはなぜでしょうか？ 色々アドバイスありがとうございます。なぜ構造体なのか？ CrystalDiskInfoの開発を開始した時点では（2008年）、SSDがこんなに増えるとは想像できていなかったこと、IDENTIFY_DEVICEデータを規格に従って分解していけば適切な対応ができると考えていたこと、そもそも10年以上続けるようなプロジェクトになるとは考えていなかったこと、その後の拡張をひたすら場当たり的に行ってきたことなどが考えられますが、今となっては理由はわかりません。 この状況で戦い続けるのは非常に厳しいので、Ver.9で0から再構築することを考えております。 |