From 9ceddd59b4eee0ca4e693f48f67c4d1596580a99 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 20 Apr 2026 14:27:43 -0400 Subject: [PATCH] Improve exposition, use protection flag. --- Storage/Disk/DiskImage/Formats/SAP.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/Storage/Disk/DiskImage/Formats/SAP.cpp b/Storage/Disk/DiskImage/Formats/SAP.cpp index c9c9367f8..ec9b0ca6f 100644 --- a/Storage/Disk/DiskImage/Formats/SAP.cpp +++ b/Storage/Disk/DiskImage/Formats/SAP.cpp @@ -69,13 +69,16 @@ std::unique_ptr SAP::track_at_position(const Track::Address address) cons for(int s = 0; s < sectors_per_track; s++) { auto §or = sectors.emplace_back(); + // Discussion suggests these two fields aren't meaningfully used, though there's some historical + // use of the protected field to mean "sector exists but wasn't loaded correctly". [[maybe_unused]] const auto format = file_.get(); - [[maybe_unused]] const auto protection = file_.get(); + const auto protection = file_.get(); + sector.address.track = file_.get(); sector.address.sector = file_.get(); sector.size = sector_size_; - // SAP uses almost-but-not-quite an ordinary CCITT CRC. + // Reread those four fields into a single block for CRC verification. file_.seek(-4, Whence::CUR); auto contents = file_.read(256 + 4); if(file_.eof()) break; @@ -84,12 +87,20 @@ std::unique_ptr SAP::track_at_position(const Track::Address address) cons for(size_t c = 4; c < contents.size(); c++) { contents[c] ^= 0xb3; } + // SAP uses almost-but-not-quite an ordinary CCITT CRC; both inputs and outputs are taken in opposite bit order. + // Also it includes the four SAP-specific fields in its CRC so this definitely isn't supposed to be the one that + // appears on the imaged disk itself even if bit order were conventional. const auto calculated = CRC::Generator::crc_of(contents); const auto crc = file_.get_be(); contents.erase(contents.begin(), contents.begin() + 4); sector.samples.push_back(contents); - sector.has_data_crc_error = crc != calculated; + sector.has_header_crc_error = calculated != crc; // From-thin-air guess: generate a header CRC error if the + // in-file CRC is amiss. To distinguish from the protected + // flag. But it's possible that the in-file CRCs actually + // come from distrust of the medium so I'm not sure about + // this at all. + sector.has_data_crc_error = protection != 0; } return Encodings::MFM::TrackWithSectors(Encodings::MFM::Density::Double, sectors);