Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/applib/storage_property_descr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ bool storage_property_autoset_description(StorageProperty& p, StorageDeviceDetec
found = auto_set(p, "ata_smart_attributes/revision", p.displayable_name.c_str());
if (!found) {
auto_set_ata_attribute_description(p, device_type);
storage_property_ata_attribute_humanize_ssd_writes(p);
found = true; // true, because auto_set_attr() may set "Unknown attribute", which is still "found".
}
break;
Expand Down
100 changes: 100 additions & 0 deletions src/applib/storage_property_descr_ata_attribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ License: GNU General Public License v3.0 only
//#include "warning_colors.h"
#include "storage_property_descr_helpers.h"
#include "hz/string_num.h"
#include "hz/format_unit.h" // format_size


namespace {
Expand Down Expand Up @@ -1364,5 +1365,104 @@ void storage_property_ata_attribute_autoset_warning(StorageProperty& p)



void storage_property_ata_attribute_humanize_ssd_writes(StorageProperty& p)
{
if (p.section != StoragePropertySection::AtaAttributes || !p.is_value_type<AtaStorageAttribute>()) {
return;
}

const auto& attr = p.get_value<AtaStorageAttribute>();

// Skip if readable_value is already set (e.g., by parser or for GiB attributes)
if (!p.readable_value.empty()) {
return;
}

// Standard sector size (512 bytes)
constexpr uint64_t bytes_per_sector = 512;
constexpr uint64_t mib_32 = 32ULL * 1024ULL * 1024ULL;
constexpr uint64_t gib = 1024ULL * 1024ULL * 1024ULL;

// Match attribute by ID and reported name to handle vendor-specific attributes
const int32_t id = attr.id;
const std::string& name = p.reported_name;
std::optional<uint64_t> bytes;

// Write attributes - these need humanization most
// Attribute 199: Write_Sectors_Tot_Ct (Indilinx Barefoot SSDs)
// Total count of written sectors
if (id == 199 && name == "Write_Sectors_Tot_Ct") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * bytes_per_sector;
}
Comment on lines +1394 to +1396

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

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

attr.raw_value_int is int64_t; casting a negative value to uint64_t before multiplying will wrap and produce an enormous byte count (and bytes > 0 will still pass). It would be safer to explicitly require attr.raw_value_int > 0 before conversion and to guard against uint64_t overflow when multiplying by sector/32MiB/1GiB (e.g., via a checked multiply or max()/multiplier pre-check).

Copilot uses AI. Check for mistakes.
// Attribute 225: Host_Writes_32MiB (Intel SSDs)
else if (id == 225 && name == "Host_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 241: Host_Writes_32MiB (various SSDs)
// Raw value increased by 1 for every 32 MiB written
else if (id == 241 && name == "Host_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 243: Host_Writes_32MiB (SanDisk SSDs)
else if (id == 243 && name == "Host_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 245: Flash_Writes_32MiB (Innodisk SSDs)
else if (id == 245 && name == "Flash_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 245: TLC_Writes_32MiB (SiliconMotion SSDs)
else if (id == 245 && name == "TLC_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
Comment on lines +1397 to +1417

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

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

The long if/else if chain duplicates the same conversion logic for multiple (id,name) pairs (e.g., several Host_*_32MiB variants). Consider consolidating this into a small static table (id + name + multiplier) and a single lookup, which will reduce the chance of missing an entry or introducing inconsistent multipliers as this list grows.

Suggested change
// Attribute 225: Host_Writes_32MiB (Intel SSDs)
else if (id == 225 && name == "Host_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 241: Host_Writes_32MiB (various SSDs)
// Raw value increased by 1 for every 32 MiB written
else if (id == 241 && name == "Host_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 243: Host_Writes_32MiB (SanDisk SSDs)
else if (id == 243 && name == "Host_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 245: Flash_Writes_32MiB (Innodisk SSDs)
else if (id == 245 && name == "Flash_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 245: TLC_Writes_32MiB (SiliconMotion SSDs)
else if (id == 245 && name == "TLC_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attributes whose raw value is a count of 32 MiB units
else {
struct WriteUnits32MiBEntry {
int32_t id;
const char* name;
};
// Raw value increased by 1 for every 32 MiB written
static constexpr WriteUnits32MiBEntry write_32mib_entries[] = {
// Attribute 225: Host_Writes_32MiB (Intel SSDs)
{225, "Host_Writes_32MiB"},
// Attribute 241: Host_Writes_32MiB (various SSDs)
{241, "Host_Writes_32MiB"},
// Attribute 243: Host_Writes_32MiB (SanDisk SSDs)
{243, "Host_Writes_32MiB"},
// Attribute 245: Flash_Writes_32MiB (Innodisk SSDs)
{245, "Flash_Writes_32MiB"},
// Attribute 245: TLC_Writes_32MiB (SiliconMotion SSDs)
{245, "TLC_Writes_32MiB"},
// Attribute 246: SLC_Writes_32MiB (SiliconMotion SSDs)
{246, "SLC_Writes_32MiB"},
};
for (const auto& entry : write_32mib_entries) {
if (id == entry.id && name == entry.name) {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
break;
}
}
}

Copilot uses AI. Check for mistakes.
// Attribute 246: SLC_Writes_32MiB (SiliconMotion SSDs)
else if (id == 246 && name == "SLC_Writes_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 246: Total_Host_Sector_Write (Crucial/Micron SSDs)
// Total number of sectors written by the host system
else if (id == 246 && name == "Total_Host_Sector_Write") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * bytes_per_sector;
}
// Attribute 249: NAND_Writes_1GiB (Intel SSDs)
// Note: The raw value is the count, not already in GiB
else if (id == 249 && name == "NAND_Writes_1GiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * gib;
}
// Attribute 249: Total_NAND_Prog_Ct_GiB (OCZ SSDs)
else if (id == 249 && name == "Total_NAND_Prog_Ct_GiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * gib;
}

// Read attributes - also humanize for consistency
// Attribute 198: Read_Sectors_Tot_Ct (Indilinx Barefoot SSDs)
else if (id == 198 && name == "Read_Sectors_Tot_Ct") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * bytes_per_sector;
}
// Attribute 226: Host_Reads_32MiB (Intel SSDs)
else if (id == 226 && name == "Host_Reads_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 242: Host_Reads_32MiB (Intel SSDs)
else if (id == 242 && name == "Host_Reads_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 244: Flash_Reads_32MiB (Innodisk SSDs)
else if (id == 244 && name == "Flash_Reads_32MiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
}
// Attribute 251: Total_NAND_Read_Ct_GiB (OCZ SSDs)
else if (id == 251 && name == "Total_NAND_Read_Ct_GiB") {
bytes = static_cast<uint64_t>(attr.raw_value_int) * gib;
}

// Set readable_value if we determined the byte count
if (bytes.has_value() && bytes.value() > 0) {
// Use binary units (KiB, MiB, GiB, TiB) for consistency with existing attributes
p.readable_value = hz::format_size(bytes.value(), false);
}
}



/// @}
5 changes: 5 additions & 0 deletions src/applib/storage_property_descr_ata_attribute.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ void auto_set_ata_attribute_description(StorageProperty& p, StorageDeviceDetecte
void storage_property_ata_attribute_autoset_warning(StorageProperty& p);


/// Humanize SSD write statistics by converting raw values to readable byte counts.
/// Sets the readable_value field for applicable write-related attributes.
Comment on lines +29 to +30

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

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

storage_property_ata_attribute_humanize_ssd_writes() name/docs say it only humanizes write-related attributes, but the implementation also converts several read-related attributes (e.g. Read_Sectors_Tot_Ct, Host_Reads_32MiB). This mismatch makes the API misleading; consider renaming the function (and updating call sites) or splitting into separate read/write helpers, and update the header comment accordingly.

Suggested change
/// Humanize SSD write statistics by converting raw values to readable byte counts.
/// Sets the readable_value field for applicable write-related attributes.
/// Humanize SSD I/O statistics (both read- and write-related) by converting raw
/// values to readable byte counts. Sets the readable_value field for applicable
/// SSD attributes.

Copilot uses AI. Check for mistakes.
void storage_property_ata_attribute_humanize_ssd_writes(StorageProperty& p);


#endif

/// @}
Loading