From 821170de2f1e0460ee74d405a17985723fa2bdeb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 13:11:19 +0000 Subject: [PATCH 1/8] fat12: harden writes and add a core test suite - Add tests/ (self-contained harness, `ctest`) and CMake options MSXFF_BUILD_GUI / MSXFF_BUILD_TESTS so core+CLI build without SDL2/GL. - DiskImage::loadBytes() loads an image from memory. - Mount: validate the BPB; fall back to the media byte in the FAT for boot sectors without a usable BPB; clamp clusters to FAT capacity. - Writes check free space first: a disk-full write or create no longer leaks clusters, truncates the file or leaves a 0-byte phantom entry. - Validate 8.3 names (too-long names were silently truncated, creating duplicates); rename checks for collisions. - Subdirectories grow past one cluster; new removeDir() and lookup(). - New/modified entries get a date and time (fixed stamp for tests). - fsck and the cluster map no longer recurse forever on directory loops; fsck reports size/chain-length mismatches and bad markers. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CtBtjm9PRm6sDhDVJbtciC --- CMakeLists.txt | 12 ++ core/disk_image.cpp | 39 +++-- core/disk_image.h | 4 + core/fat12.cpp | 345 +++++++++++++++++++++++++++++++-------- core/fat12.h | 25 ++- tests/CMakeLists.txt | 4 + tests/check.h | 44 +++++ tests/test_basic.cpp | 1 + tests/test_disk.cpp | 1 + tests/test_fat12.cpp | 377 +++++++++++++++++++++++++++++++++++++++++++ tests/test_main.cpp | 32 ++++ tests/test_rom.cpp | 1 + tests/test_tape.cpp | 1 + 13 files changed, 800 insertions(+), 86 deletions(-) create mode 100644 tests/CMakeLists.txt create mode 100644 tests/check.h create mode 100644 tests/test_basic.cpp create mode 100644 tests/test_disk.cpp create mode 100644 tests/test_fat12.cpp create mode 100644 tests/test_main.cpp create mode 100644 tests/test_rom.cpp create mode 100644 tests/test_tape.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index f9f458e..08746e9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,7 +20,18 @@ target_link_libraries(msxforge PRIVATE msxcore) # --- dsk2dmk test tool --- add_executable(dsk2dmk tools/dsk2dmk.cpp) +# --- tests (core library, no GUI deps): ctest --test-dir build --- +option(MSXFF_BUILD_TESTS "Build the core test suite" ON) +if(MSXFF_BUILD_TESTS) + enable_testing() + add_subdirectory(tests) +endif() + # --- GUI (Dear ImGui + SDL2 + OpenGL3 + tinyfiledialogs) --- +# Turn off with -DMSXFF_BUILD_GUI=OFF to build only the core, CLI and tests +# (no SDL2 / OpenGL needed: handy for headless CI and servers). +option(MSXFF_BUILD_GUI "Build the Dear ImGui desktop app" ON) +if(MSXFF_BUILD_GUI) set(IMGUI ${CMAKE_SOURCE_DIR}/libs/imgui) add_executable(msxforge-gui gui/main.cpp @@ -56,3 +67,4 @@ else() target_link_libraries(msxforge-gui PRIVATE ${SDL2_LIBRARIES}) endif() endif() +endif() # MSXFF_BUILD_GUI diff --git a/core/disk_image.cpp b/core/disk_image.cpp index d449afb..ab30384 100644 --- a/core/disk_image.cpp +++ b/core/disk_image.cpp @@ -43,27 +43,36 @@ static bool hasExt(const std::string& p, const char* ext) { } bool DiskImage::load(const std::string& path, std::string* err) { - path_ = path; - format_ = DiskFormat::Unknown; - raw_.clear(); sectors_.clear(); lba_.clear(); - cylinders_ = heads_ = spt_ = 0; protectedFlags_ = false; - - if (!readWhole(path, raw_)) { if (err) *err = "cannot read file"; return false; } - - // DMK is identified by extension + a plausible 16-byte header. Everything - // else is treated as a flat sector dump. - bool looksDmk = hasExt(path, ".dmk"); - if (!looksDmk && raw_.size() >= 16) { - // A DMK track length at [2..3] that divides the body cleanly is a hint, - // but we only auto-detect on extension to avoid mis-reading .dsk files. + std::vector bytes; + if (!readWhole(path, bytes)) { + path_ = path; format_ = DiskFormat::Unknown; + raw_.clear(); sectors_.clear(); lba_.clear(); + if (err) *err = "cannot read file"; + return false; } + return loadBytes(std::move(bytes), path, err); +} + +bool DiskImage::loadBytes(std::vector bytes, const std::string& nameHint, + std::string* err) { + path_ = nameHint; + format_ = DiskFormat::Unknown; + raw_ = std::move(bytes); + sectors_.clear(); lba_.clear(); + cylinders_ = heads_ = spt_ = 0; protectedFlags_ = false; dirty_ = false; + if (err) err->clear(); - if (looksDmk) { + // DMK is identified by extension only, to avoid mis-reading .dsk files + // whose first bytes happen to look like a DMK header. + if (hasExt(nameHint, ".dmk")) { if (parseDmk(err)) { format_ = DiskFormat::DMK; return true; } // Fall through: some ".dmk" files are actually plain dumps. + sectors_.clear(); lba_.clear(); + cylinders_ = heads_ = spt_ = 0; protectedFlags_ = false; } - if (parsePlain(err)) return true; + if (parsePlain(err)) { if (err) err->clear(); return true; } if (err && err->empty()) *err = "unrecognized disk image"; + raw_.clear(); return false; } diff --git a/core/disk_image.h b/core/disk_image.h index 2019590..7737798 100644 --- a/core/disk_image.h +++ b/core/disk_image.h @@ -42,6 +42,10 @@ struct SectorInfo { class DiskImage { public: bool load(const std::string& path, std::string* err = nullptr); + // Same as load() but from bytes already in memory. `nameHint` is only used + // for its extension (".dmk" selects the track-image parser). + bool loadBytes(std::vector bytes, const std::string& nameHint, + std::string* err = nullptr); bool loaded() const { return !raw_.empty(); } DiskFormat format() const { return format_; } diff --git a/core/fat12.cpp b/core/fat12.cpp index 78e0688..fac265b 100644 --- a/core/fat12.cpp +++ b/core/fat12.cpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include namespace msx { @@ -55,6 +57,40 @@ bool Fat12::sectorBytes(long lba, uint8_t out[512]) const { return disk_ && disk_->readSector(lba, out); } +// Standard MSX/PC floppy geometries, keyed by the media descriptor byte that +// also opens the FAT. Used when the boot sector carries no usable BPB (some +// MSX-DOS 1 era disks and many game disks with custom boot code). +namespace { +struct MediaGeo { uint8_t media; int spc, ndir, spf, total; }; +const MediaGeo kMediaGeo[] = { + {0xF8, 2, 112, 2, 720}, // 360K SS, 80 tracks, 9 spt + {0xF9, 2, 112, 3, 1440}, // 720K DS, 80 tracks, 9 spt + {0xFA, 2, 112, 1, 640}, // 320K SS, 80 tracks, 8 spt + {0xFB, 2, 112, 2, 1280}, // 640K DS, 80 tracks, 8 spt + {0xFC, 1, 64, 2, 360}, // 180K SS, 40 tracks, 9 spt + {0xFD, 2, 112, 2, 720}, // 360K DS, 40 tracks, 9 spt + {0xFE, 1, 64, 1, 320}, // 160K SS, 40 tracks, 8 spt + {0xFF, 2, 112, 1, 640}, // 320K DS, 40 tracks, 8 spt +}; +bool isPow2(int v) { return v > 0 && (v & (v - 1)) == 0; } +} // namespace + +bool Fat12::mountFromMediaByte(std::string* err) { + uint8_t f[512]; + if (!sectorBytes(1, f) || f[1] != 0xFF || f[2] != 0xFF) { + if (err) *err = "not a FAT12 boot sector"; + return false; + } + for (const auto& g : kMediaGeo) { + if (g.media != f[0]) continue; + bps_ = 512; spc_ = g.spc; rsvd_ = 1; nfat_ = 2; ndir_ = g.ndir; + spf_ = g.spf; totalSectors_ = g.total; media_ = g.media; + return true; + } + if (err) *err = "not a FAT12 boot sector (unknown media byte)"; + return false; +} + bool Fat12::mount(DiskImage& disk, std::string* err) { disk_ = &disk; mounted_ = false; @@ -71,11 +107,17 @@ bool Fat12::mount(DiskImage& disk, std::string* err) { spf_ = rd16(b + 0x16); oem_ = trim(std::string((const char*)b + 3, 8)); - if (bps_ != 512 || spc_ == 0 || nfat_ == 0 || ndir_ == 0 || spf_ == 0 || rsvd_ == 0) { - if (err) *err = "not a FAT12 boot sector"; - return false; + bool bpbOk = bps_ == 512 && isPow2(spc_) && spc_ <= 64 && nfat_ >= 1 && nfat_ <= 4 && + ndir_ > 0 && ndir_ % 16 == 0 && spf_ > 0 && rsvd_ > 0; + if (bpbOk && totalSectors_ == 0) totalSectors_ = (int)disk.logicalSectors(); + if (bpbOk) { + long data = (long)totalSectors_ - rsvd_ - (long)nfat_ * spf_ - (ndir_ * 32 + 511) / 512; + bpbOk = data >= spc_; + } + if (!bpbOk) { + oem_.clear(); + if (!mountFromMediaByte(err)) return false; } - if (totalSectors_ == 0) totalSectors_ = (int)disk.logicalSectors(); fatStartSec_ = rsvd_; rootStartSec_ = rsvd_ + (long)nfat_ * spf_; @@ -83,13 +125,18 @@ bool Fat12::mount(DiskImage& disk, std::string* err) { dataStartSec_ = rootStartSec_ + rootSectors_; long dataSectors = totalSectors_ - dataStartSec_; totalClusters_ = spc_ > 0 ? (int)(dataSectors / spc_) : 0; + // A FAT12 entry is 1.5 bytes: never address clusters the FAT cannot hold. + int fatCapacity = (int)((long)spf_ * 512 * 2 / 3) - 2; + if (totalClusters_ > fatCapacity) totalClusters_ = fatCapacity; + if (totalClusters_ > 4084) totalClusters_ = 4084; + if (totalClusters_ < 1) { if (err) *err = "FAT12: no data area"; return false; } // Volume label: prefer a root VOL entry, else BPB ext label if present. volLabel_.clear(); for (auto& e : readRootDir(false)) { if (e.isVolume()) { volLabel_ = e.name; break; } } - if (volLabel_.empty() && b[0x26] == 0x29) // extended boot sig + if (volLabel_.empty() && bpbOk && b[0x26] == 0x29) // extended boot sig volLabel_ = trim(std::string((const char*)b + 0x2B, 11)); mounted_ = true; @@ -168,13 +215,26 @@ std::vector Fat12::readDirAtCluster(int cluster, bool includeDeleted) return parseEntries(buf, firstBase, includeDeleted); } -const DirEntry* Fat12::resolve(const std::string& path) const { - // Walk the path components; returns a heap-lived copy is awkward, so we keep - // a static thread-local last-resolved entry. Simpler: return by storing. - // Here we resolve into a cached member via a small trick: not needed because - // list() re-resolves. This helper is unused for root. - (void)path; - return nullptr; +std::vector Fat12::readDir(int dirCluster, bool includeDeleted) const { + return dirCluster == 0 ? readRootDir(includeDeleted) : readDirAtCluster(dirCluster, includeDeleted); +} + +bool Fat12::lookup(const std::string& path, DirEntry& out) const { + if (!mounted_) return false; + size_t cut = path.find_last_of("/\\"); + std::string dir = (cut == std::string::npos) ? "/" : path.substr(0, cut); + std::string leaf = (cut == std::string::npos) ? path : path.substr(cut + 1); + if (leaf.empty()) return false; + int dc = resolveDirCluster(dir); + if (dc < 0) return false; + for (char& c : leaf) c = (char)toupper((unsigned char)c); + for (auto& e : readDir(dc, false)) { + if (e.isVolume()) continue; + std::string nm = e.name; + for (char& c : nm) c = (char)toupper((unsigned char)c); + if (nm == leaf) { out = e; return true; } + } + return false; } std::vector Fat12::list(const std::string& path, bool includeDeleted) const { @@ -311,26 +371,50 @@ bool Fat12::writeClusterData(int cluster, const uint8_t* src, long len) { return true; } +int Fat12::countFreeClusters() const { + int n = 0; + for (int c = 2; c < totalClusters_ + 2; ++c) + if (fatEntry(c) == 0) n++; + return n; +} + +void Fat12::stamp(uint16_t& date, uint16_t& time) const { + if (fixedStamp_) { date = fixedDate_; time = fixedTime_; return; } + std::time_t now = std::time(nullptr); + std::tm tmv{}; +#if defined(_WIN32) + localtime_s(&tmv, &now); +#else + localtime_r(&now, &tmv); +#endif + int y = tmv.tm_year + 1900; + if (y < 1980) y = 1980; + if (y > 2107) y = 2107; + date = (uint16_t)(((y - 1980) << 9) | ((tmv.tm_mon + 1) << 5) | tmv.tm_mday); + time = (uint16_t)((tmv.tm_hour << 11) | (tmv.tm_min << 5) | (tmv.tm_sec / 2)); +} + bool Fat12::writeFile(DirEntry& e, const std::vector& data, std::string* err) { if (!mounted_) { if (err) *err = "not mounted"; return false; } if (e.isDir()) { if (err) *err = "cannot overwrite a directory"; return false; } long clusterBytes = (long)spc_ * 512; - int needClusters = data.empty() ? 0 : (int)((data.size() + clusterBytes - 1) / clusterBytes); - - std::vector have = (e.startCluster >= 2) ? chainOf(e.startCluster) : std::vector{}; - - // Grow the chain if needed by allocating free clusters. - std::vector chain = have; - if ((int)chain.size() < needClusters) { - // Temporarily reserve as we go so allocCluster doesn't return dupes. - int extra = needClusters - (int)chain.size(); - for (int i = 0; i < extra; ++i) { - int nc = allocCluster(); - if (nc < 0) { if (err) *err = "disk full (no free clusters)"; return false; } - setFatEntry(nc, 0xFFF); // reserve immediately - chain.push_back(nc); - } + long needL = data.empty() ? 0 : (long)((data.size() + clusterBytes - 1) / clusterBytes); + std::vector chain = (e.startCluster >= 2) ? chainOf(e.startCluster) : std::vector{}; + + // Check the space up front so a failed write leaves the disk untouched + // (no half-allocated clusters, no truncated file). + if (needL > (long)chain.size() && + needL - (long)chain.size() > (long)countFreeClusters()) { + if (err) *err = "disk full (no free clusters)"; + return false; + } + int needClusters = (int)needL; + while ((int)chain.size() < needClusters) { + int nc = allocCluster(); + if (nc < 0) { if (err) *err = "disk full (no free clusters)"; return false; } + setFatEntry(nc, 0xFFF); // reserve immediately so the next alloc skips it + chain.push_back(nc); } // Write the payload across the (first needClusters) clusters. @@ -338,7 +422,8 @@ bool Fat12::writeFile(DirEntry& e, const std::vector& data, std::string long off = (long)i * clusterBytes; long len = (long)data.size() - off; if (len > clusterBytes) len = clusterBytes; if (!writeClusterData(chain[i], data.data() + off, len)) { - if (err) *err = "write error"; return false; + if (err) *err = "write error"; + return false; } } @@ -348,12 +433,17 @@ bool Fat12::writeFile(DirEntry& e, const std::vector& data, std::string setFatEntry(chain[i], next); } for (size_t i = needClusters; i < chain.size(); ++i) - setFatEntry(chain[i], 0); // free surplus (both old chain tail + over-alloc) + setFatEntry(chain[i], 0); // free the surplus tail of the old chain - // Update the directory entry (size + start cluster). + // Update the directory entry (size, start cluster, modification time). uint16_t newStart = (needClusters > 0) ? (uint16_t)chain[0] : 0; uint32_t newSize = (uint32_t)data.size(); + uint16_t date, time; stamp(date, time); if (e.entryOffset >= 0) { + writeByteAt(e.entryOffset + 0x16, (uint8_t)(time & 0xFF)); + writeByteAt(e.entryOffset + 0x17, (uint8_t)(time >> 8)); + writeByteAt(e.entryOffset + 0x18, (uint8_t)(date & 0xFF)); + writeByteAt(e.entryOffset + 0x19, (uint8_t)(date >> 8)); writeByteAt(e.entryOffset + 0x1A, (uint8_t)(newStart & 0xFF)); writeByteAt(e.entryOffset + 0x1B, (uint8_t)(newStart >> 8)); writeByteAt(e.entryOffset + 0x1C, (uint8_t)(newSize & 0xFF)); @@ -363,14 +453,32 @@ bool Fat12::writeFile(DirEntry& e, const std::vector& data, std::string } e.startCluster = newStart; e.size = newSize; + e.date = date; e.time = time; return true; } bool Fat12::deleteFile(const DirEntry& e, std::string* err) { if (!mounted_) { if (err) *err = "not mounted"; return false; } - if (e.isDir()) { if (err) *err = "refusing to delete a directory"; return false; } + if (e.isDir()) { if (err) *err = "refusing to delete a directory (use removeDir)"; return false; } + if (e.deleted || e.entryOffset < 0) { if (err) *err = "entry is not a live file"; return false; } for (int c : chainOf(e.startCluster)) setFatEntry(c, 0); - if (e.entryOffset >= 0) writeByteAt(e.entryOffset, 0xE5); // mark deleted + writeByteAt(e.entryOffset, 0xE5); // mark deleted + return true; +} + +bool Fat12::removeDir(const DirEntry& e, std::string* err) { + if (!mounted_) { if (err) *err = "not mounted"; return false; } + if (!e.isDir() || e.deleted || e.entryOffset < 0 || e.startCluster < 2) { + if (err) *err = "not a subdirectory"; + return false; + } + for (auto& c : readDirAtCluster(e.startCluster, false)) { + if (c.name == "." || c.name == "..") continue; + if (err) *err = "directory is not empty"; + return false; + } + for (int c : chainOf(e.startCluster)) setFatEntry(c, 0); + writeByteAt(e.entryOffset, 0xE5); return true; } @@ -381,15 +489,22 @@ long Fat12::clusterByteOffset(int cluster) const { } void Fat12::collectNamedClusters(int cluster, std::vector& status, bool isRoot) const { - std::vector entries = isRoot ? readRootDir(false) : readDirAtCluster(cluster, false); - for (auto& e : entries) { - if (e.name == "." || e.name == ".." || e.deleted) continue; // deleted stay "raw" - if (e.isVolume()) continue; - for (int c : chainOf(e.startCluster)) { - int idx = c - 2; - if (idx >= 0 && idx < (int)status.size()) status[idx] = CL_NAMED; + // Walk the tree breadth-first; `seen` stops corrupt directories that link + // back to an ancestor (or themselves) from looping forever. + std::set seen; + std::vector todo{isRoot ? 0 : cluster}; + while (!todo.empty()) { + int dc = todo.back(); todo.pop_back(); + if (!seen.insert(dc).second) continue; + for (auto& e : readDir(dc, false)) { + if (e.name == "." || e.name == ".." || e.deleted) continue; // deleted stay "raw" + if (e.isVolume()) continue; + for (int c : chainOf(e.startCluster)) { + int idx = c - 2; + if (idx >= 0 && idx < (int)status.size()) status[idx] = CL_NAMED; + } + if (e.isDir() && e.startCluster >= 2) todo.push_back(e.startCluster); } - if (e.isDir() && e.startCluster >= 2) collectNamedClusters(e.startCluster, status, false); } } @@ -454,6 +569,26 @@ bool Fat12::writeRawUnnamed(const std::vector& data, long* written, std // ---- Directory writes: create / rename / mkdir ---------------------------- +bool Fat12::validName83(const std::string& name, std::string* err) { + auto bad = [&](const char* why) { if (err) *err = std::string("invalid 8.3 name '") + name + "': " + why; return false; }; + if (name.empty()) return bad("empty"); + size_t dot = name.find('.'); + if (dot != std::string::npos && name.find('.', dot + 1) != std::string::npos) return bad("more than one dot"); + std::string base = (dot == std::string::npos) ? name : name.substr(0, dot); + std::string ext = (dot == std::string::npos) ? "" : name.substr(dot + 1); + if (base.empty()) return bad("empty base name"); + if (base.size() > 8) return bad("base name longer than 8 characters"); + if (ext.size() > 3) return bad("extension longer than 3 characters"); + if (dot != std::string::npos && ext.empty()) return bad("empty extension after the dot"); + static const char* kReserved = "\"*+,./:;<=>?[\\]| "; + for (char ch : base + ext) { + unsigned char c = (unsigned char)ch; + if (c < 0x20 || c == 0x7F || strchr(kReserved, c)) return bad("reserved character"); + } + if ((unsigned char)base[0] == 0xE5) return bad("first character 0xE5 marks deleted entries"); + return true; +} + void Fat12::pack83(const std::string& name, uint8_t out[11]) { memset(out, ' ', 11); if (name == ".") { out[0] = '.'; return; } @@ -465,6 +600,14 @@ void Fat12::pack83(const std::string& name, uint8_t out[11]) { for (size_t i = 0; i < ext.size() && i < 3; ++i) out[8 + i] = (uint8_t)toupper((unsigned char)ext[i]); } +bool Fat12::nameTaken(int dirCluster, const uint8_t name11[11], long ignoreOffset) const { + for (auto& e : readDir(dirCluster, false)) { + if (e.entryOffset == ignoreOffset || e.raw83.size() != 11) continue; + if (memcmp(e.raw83.data(), name11, 11) == 0) return true; + } + return false; +} + int Fat12::resolveDirCluster(const std::string& path) const { std::vector parts; std::string cur; for (char c : path) { if (c == '/' || c == '\\') { if (!cur.empty()) { parts.push_back(cur); cur.clear(); } } else cur.push_back(c); } @@ -494,10 +637,11 @@ long Fat12::freeDirSlot(int dirCluster) { uint8_t b; if (!readByteAt(off, b)) return -1; if (b == 0x00 || b == 0xE5) return off; } - return -1; + return -1; // the root directory has a fixed size } int slotsPerCluster = spc_ * 512 / 32; - for (int c : chainOf(dirCluster)) { + std::vector chain = chainOf(dirCluster); + for (int c : chain) { long firstSec = dataStartSec_ + (long)(c - 2) * spc_; for (int s = 0; s < slotsPerCluster; ++s) { long off = firstSec * 512 + (long)s * 32; @@ -505,7 +649,15 @@ long Fat12::freeDirSlot(int dirCluster) { if (b == 0x00 || b == 0xE5) return off; } } - return -1; + // Subdirectories grow: append a zeroed cluster to the directory's chain. + if (chain.empty()) return -1; + int nc = allocCluster(); + if (nc < 0) return -1; + std::vector zero((size_t)spc_ * 512, 0); + if (!writeClusterData(nc, zero.data(), (long)zero.size())) return -1; + setFatEntry(nc, 0xFFF); + setFatEntry(chain.back(), nc); + return clusterByteOffset(nc); } void Fat12::writeDirEntry(long off, const uint8_t name11[11], uint8_t attr, @@ -513,6 +665,9 @@ void Fat12::writeDirEntry(long off, const uint8_t name11[11], uint8_t attr, uint8_t e[32]; memset(e, 0, sizeof e); memcpy(e, name11, 11); e[0x0B] = attr; + uint16_t date, time; stamp(date, time); + e[0x16] = time & 0xFF; e[0x17] = (time >> 8) & 0xFF; + e[0x18] = date & 0xFF; e[0x19] = (date >> 8) & 0xFF; e[0x1A] = cluster & 0xFF; e[0x1B] = (cluster >> 8) & 0xFF; e[0x1C] = size & 0xFF; e[0x1D] = (size >> 8) & 0xFF; e[0x1E] = (size >> 16) & 0xFF; e[0x1F] = (size >> 24) & 0xFF; @@ -522,39 +677,67 @@ void Fat12::writeDirEntry(long off, const uint8_t name11[11], uint8_t attr, bool Fat12::createFile(const std::string& dirPath, const std::string& name, const std::vector& data, std::string* err) { if (!mounted_) { if (err) *err = "not mounted"; return false; } + if (!validName83(name, err)) return false; int dc = resolveDirCluster(dirPath); if (dc < 0) { if (err) *err = "directory not found"; return false; } - // Reject a duplicate name. - std::string up = name; for (auto& c : up) c = (char)toupper((unsigned char)c); - for (auto& e : list(dirPath, false)) { - std::string nm = e.name; for (auto& c : nm) c = (char)toupper((unsigned char)c); - if (nm == up) { if (err) *err = "a file with that name already exists"; return false; } - } + uint8_t nm[11]; pack83(name, nm); + if (nameTaken(dc, nm)) { if (err) *err = "a file with that name already exists"; return false; } + + long clusterBytes = (long)spc_ * 512; + long need = (long)((data.size() + clusterBytes - 1) / clusterBytes); + if (need > countFreeClusters()) { if (err) *err = "disk full (no free clusters)"; return false; } long slot = freeDirSlot(dc); if (slot < 0) { if (err) *err = "directory is full"; return false; } - uint8_t nm[11]; pack83(name, nm); + uint8_t prevFirst = 0; readByteAt(slot, prevFirst); writeDirEntry(slot, nm, 0x20 /*archive*/, 0, 0); DirEntry tmp; tmp.entryOffset = slot; tmp.startCluster = 0; tmp.size = 0; tmp.attr = 0x20; - return writeFile(tmp, data, err); // allocates clusters + updates size/start + if (!writeFile(tmp, data, err)) { + // Roll back the directory entry so no phantom 0-byte file is left. + uint8_t blank[32]; memset(blank, 0, sizeof blank); + blank[0] = prevFirst == 0xE5 ? 0xE5 : 0x00; + for (int i = 0; i < 32; ++i) writeByteAt(slot + i, blank[i]); + return false; + } + return true; } bool Fat12::renameEntry(DirEntry& e, const std::string& newName, std::string* err) { - if (!mounted_ || e.entryOffset < 0) { if (err) *err = "cannot rename"; return false; } + if (!mounted_ || e.entryOffset < 0 || e.deleted) { if (err) *err = "cannot rename"; return false; } + if (!validName83(newName, err)) return false; uint8_t nm[11]; pack83(newName, nm); + // The entry's own directory: find it by scanning for the record offset. + long dirOf = -1; + std::set seen; + std::vector todo{0}; + while (!todo.empty() && dirOf < 0) { + int dc = todo.back(); todo.pop_back(); + if (!seen.insert(dc).second) continue; + for (auto& x : readDir(dc, false)) { + if (x.entryOffset == e.entryOffset) { dirOf = dc; break; } + if (x.isDir() && x.startCluster >= 2 && x.name != "." && x.name != "..") + todo.push_back(x.startCluster); + } + } + if (dirOf >= 0 && nameTaken((int)dirOf, nm, e.entryOffset)) { + if (err) *err = "a file with that name already exists"; + return false; + } for (int i = 0; i < 11; ++i) writeByteAt(e.entryOffset + i, nm[i]); e.raw83.assign((const char*)nm, 11); + std::string base((const char*)nm, 8), ext((const char*)nm + 8, 3); + base = trim(base); ext = trim(ext); + e.name = ext.empty() ? base : base + "." + ext; return true; } bool Fat12::makeDir(const std::string& parentPath, const std::string& name, std::string* err) { if (!mounted_) { if (err) *err = "not mounted"; return false; } + if (!validName83(name, err)) return false; int parent = resolveDirCluster(parentPath); if (parent < 0) { if (err) *err = "parent directory not found"; return false; } - std::string up = name; for (auto& c : up) c = (char)toupper((unsigned char)c); - for (auto& e : list(parentPath, false)) { - std::string nm = e.name; for (auto& c : nm) c = (char)toupper((unsigned char)c); - if (nm == up) { if (err) *err = "name already exists"; return false; } - } + uint8_t nm[11]; pack83(name, nm); + if (nameTaken(parent, nm)) { if (err) *err = "name already exists"; return false; } + if (countFreeClusters() < (parent != 0 ? 2 : 1)) { if (err) *err = "disk full"; return false; } long slot = freeDirSlot(parent); if (slot < 0) { if (err) *err = "parent directory is full"; return false; } int nc = allocCluster(); @@ -568,7 +751,6 @@ bool Fat12::makeDir(const std::string& parentPath, const std::string& name, std: writeDirEntry(firstSec * 512 + 0, dot, 0x10, (uint16_t)nc, 0); writeDirEntry(firstSec * 512 + 32, dotdot, 0x10, (uint16_t)parent, 0); // Add the entry to the parent. - uint8_t nm[11]; pack83(name, nm); writeDirEntry(slot, nm, 0x10, (uint16_t)nc, 0); return true; } @@ -579,29 +761,52 @@ Fat12::FsCheck Fat12::verify() const { FsCheck r; if (!mounted_) { r.issues.push_back("filesystem not mounted"); return r; } std::vector owner(totalClusters_ > 0 ? totalClusters_ : 0, -1); - // Walk every named file/dir chain, flagging bad pointers and cross-links. - std::function walk = - [&](int cluster, bool isRoot, const std::string& base) { - for (auto& e : (isRoot ? readRootDir(false) : readDirAtCluster(cluster, false))) { + long clusterBytes = (long)spc_ * 512; + std::set seenDirs; + // Walk every named file/dir chain, flagging bad pointers, cross-links and + // sizes that do not match the chain length. + std::function walk = [&](int dirCluster, const std::string& base) { + if (!seenDirs.insert(dirCluster).second) { + r.issues.push_back((base.empty() ? "/" : base) + ": directory loop"); + return; + } + for (auto& e : readDir(dirCluster, false)) { if (e.name == "." || e.name == ".." || e.deleted || e.isVolume()) continue; std::string path = base + "/" + e.name; int c = e.startCluster, guard = 0; - while (c >= 2 && c < 0xFF8 && guard++ < totalClusters_ + 4) { + long chainLen = 0; + bool broken = false; + if (c == 1 || (c >= 2 && c - 2 >= totalClusters_ && c < 0xFF8)) { + r.issues.push_back(path + ": start cluster out of range"); broken = true; + } + while (!broken && c >= 2 && c < 0xFF8 && guard++ < totalClusters_ + 4) { int idx = c - 2; - if (idx < 0 || idx >= (int)owner.size()) { r.issues.push_back(path + ": cluster out of range"); break; } - if (owner[idx] >= 0) { r.issues.push_back(path + ": cross-linked at cluster " + std::to_string(c)); break; } - owner[idx] = 1; r.usedByFiles++; + if (idx >= (int)owner.size()) { r.issues.push_back(path + ": cluster out of range"); broken = true; break; } + if (owner[idx] >= 0) { r.issues.push_back(path + ": cross-linked at cluster " + std::to_string(c)); broken = true; break; } + owner[idx] = 1; r.usedByFiles++; chainLen++; int nxt = fatEntry(c); - if (nxt == 0) { r.issues.push_back(path + ": chain hits a free cluster"); break; } + if (nxt == 0) { r.issues.push_back(path + ": chain hits a free cluster"); broken = true; break; } + if (nxt == 1 || (nxt >= 0xFF0 && nxt <= 0xFF7)) { + r.issues.push_back(path + ": chain hits a reserved/bad cluster marker"); broken = true; break; + } c = nxt; } - if (e.isDir() && e.startCluster >= 2) walk(e.startCluster, false, path); + if (!broken && !e.isDir()) { + long want = ((long)e.size + clusterBytes - 1) / clusterBytes; + if (want != chainLen) + r.issues.push_back(path + ": size " + std::to_string(e.size) + " needs " + + std::to_string(want) + " cluster(s) but the chain has " + + std::to_string(chainLen)); + } + if (e.isDir() && e.startCluster >= 2) walk(e.startCluster, path); } }; - walk(0, true, ""); + walk(0, ""); for (int c = 2; c < totalClusters_ + 2; ++c) { int idx = c - 2; - if (fatEntry(c) == 0) r.freeClusters++; + int v = fatEntry(c); + if (v == 0) r.freeClusters++; + else if (v == 0xFF7) continue; // marked bad: neither free nor lost else if (idx >= 0 && idx < (int)owner.size() && owner[idx] < 0) r.unreferenced++; } return r; diff --git a/core/fat12.h b/core/fat12.h index 26b5e35..2c82944 100644 --- a/core/fat12.h +++ b/core/fat12.h @@ -74,6 +74,23 @@ class Fat12 { bool createFile(const std::string& dirPath, const std::string& name, const std::vector& data, std::string* err = nullptr); + // Remove an EMPTY subdirectory (only "." and ".." left) and free its chain. + bool removeDir(const DirEntry& e, std::string* err = nullptr); + + // Find a file or directory by path ("/GAMES/SAVE/SLOT1.DAT", case-insensitive). + // Returns false if any component is missing. + bool lookup(const std::string& path, DirEntry& out) const; + + // True if `name` is a legal MSX-DOS 8.3 name (1-8 char base, optional 1-3 + // char extension, no spaces or reserved characters). Case-insensitive. + static bool validName83(const std::string& name, std::string* err = nullptr); + + // Timestamp written into new/modified directory entries (FAT date/time + // words). By default the current local time is used; a fixed stamp makes + // output reproducible (tests, deterministic builds). + void setTimestamp(uint16_t date, uint16_t time) { fixedDate_ = date; fixedTime_ = time; fixedStamp_ = true; } + void useCurrentTime() { fixedStamp_ = false; } + // Rename an entry in place (8.3). Updates `e`. bool renameEntry(DirEntry& e, const std::string& newName, std::string* err = nullptr); @@ -121,6 +138,11 @@ class Fat12 { long freeDirSlot(int dirCluster); // image offset or -1 void writeDirEntry(long off, const uint8_t name11[11], uint8_t attr, uint16_t cluster, uint32_t size); + void stamp(uint16_t& date, uint16_t& time) const; + int countFreeClusters() const; + bool nameTaken(int dirCluster, const uint8_t name11[11], long ignoreOffset = -1) const; + std::vector readDir(int dirCluster, bool includeDeleted) const; + bool mountFromMediaByte(std::string* err); static void pack83(const std::string& name, uint8_t out[11]); void collectNamedClusters(int cluster, std::vector& status, bool isRoot) const; bool writeByteAt(long imageOffset, uint8_t value); @@ -133,7 +155,6 @@ class Fat12 { std::vector readRootDir(bool includeDeleted) const; int fatEntry(int cluster) const; bool sectorBytes(long lba, uint8_t out[512]) const; - const DirEntry* resolve(const std::string& path) const; static std::vector parseEntries(const std::vector& buf, long baseOffset, bool includeDeleted); @@ -145,6 +166,8 @@ class Fat12 { long fatStartSec_ = 0, rootStartSec_ = 0, dataStartSec_ = 0; int rootSectors_ = 0; std::string volLabel_, oem_; + bool fixedStamp_ = false; + uint16_t fixedDate_ = 0, fixedTime_ = 0; }; } // namespace msx diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt new file mode 100644 index 0000000..b5f8536 --- /dev/null +++ b/tests/CMakeLists.txt @@ -0,0 +1,4 @@ +add_executable(msxff_tests + test_main.cpp test_fat12.cpp test_basic.cpp test_tape.cpp test_rom.cpp test_disk.cpp) +target_link_libraries(msxff_tests PRIVATE msxcore) +add_test(NAME core COMMAND msxff_tests) diff --git a/tests/check.h b/tests/check.h new file mode 100644 index 0000000..9fc3c8c --- /dev/null +++ b/tests/check.h @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (C) 2026 Alonso (GS·RUN). MSXFileForge. +// Free software under the GNU GPL v3 (or later) - see LICENSE. Modified +// versions must be distributed under the same license. +// MSXFileForge - minimal self-registering test harness (no external deps). +#pragma once +#include +#include +#include +#include +#include +#include + +namespace msxtest { + +struct Case { const char* name; std::function fn; }; +std::vector& registry(); +void fail(const char* file, int line, const std::string& what); + +struct Reg { Reg(const char* n, std::function f) { registry().push_back({n, std::move(f)}); } }; + +template std::string show(const T& v) { + std::ostringstream os; + if constexpr (std::is_enum_v) os << "enum(" << (long long)v << ")"; + else os << v; + return os.str(); +} +inline std::string show(const std::string& s) { return "\"" + s + "\""; } +inline std::string show(const char* s) { return show(std::string(s)); } +inline std::string show(unsigned char v) { return std::to_string((int)v); } +inline std::string show(bool v) { return v ? "true" : "false"; } + +} // namespace msxtest + +#define MSXT_CAT2(a, b) a##b +#define MSXT_CAT(a, b) MSXT_CAT2(a, b) +#define TEST(name) \ + static void MSXT_CAT(test_, name)(); \ + static msxtest::Reg MSXT_CAT(reg_, name)(#name, MSXT_CAT(test_, name)); \ + static void MSXT_CAT(test_, name)() + +#define CHECK(cond) do { if (!(cond)) msxtest::fail(__FILE__, __LINE__, "CHECK(" #cond ")"); } while (0) +#define CHECK_EQ(a, b) do { auto _va = (a); auto _vb = (b); if (!(_va == _vb)) \ + msxtest::fail(__FILE__, __LINE__, "CHECK_EQ(" #a ", " #b "): " + msxtest::show(_va) + " != " + msxtest::show(_vb)); } while (0) diff --git a/tests/test_basic.cpp b/tests/test_basic.cpp new file mode 100644 index 0000000..f106c1d --- /dev/null +++ b/tests/test_basic.cpp @@ -0,0 +1 @@ +#include "check.h" diff --git a/tests/test_disk.cpp b/tests/test_disk.cpp new file mode 100644 index 0000000..f106c1d --- /dev/null +++ b/tests/test_disk.cpp @@ -0,0 +1 @@ +#include "check.h" diff --git a/tests/test_fat12.cpp b/tests/test_fat12.cpp new file mode 100644 index 0000000..d6fed96 --- /dev/null +++ b/tests/test_fat12.cpp @@ -0,0 +1,377 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (C) 2026 Alonso (GS·RUN). MSXFileForge. +// Free software under the GNU GPL v3 (or later) - see LICENSE. Modified +// versions must be distributed under the same license. +// FAT12: format, create, read, overwrite, delete, rename, mkdir, fsck. +#include "check.h" +#include "disk_image.h" +#include "fat12.h" +#include "util.h" +#include + +using namespace msx; + +namespace { + +std::vector pattern(size_t n, uint8_t seed) { + std::vector v(n); + uint32_t x = 0x9E3779B9u ^ seed; + for (auto& b : v) { x = x * 1664525u + 1013904223u; b = (uint8_t)(x >> 24); } + return v; +} + +struct Disk { + DiskImage dk; + Fat12 fs; + explicit Disk(int sectors = 1440) { + std::string err; + bool ok = dk.loadBytes(formatBlankImage(sectors), "blank.dsk", &err); + CHECK(ok); + CHECK(fs.mount(dk, &err)); + fs.setTimestamp(0x5A21, 0x6000); // 2025-01-01 12:00:00, deterministic + } + // Re-mount from the current bytes, as a fresh reader would see them. + void remount() { + std::vector bytes = dk.raw(); + std::string err; + CHECK(dk.loadBytes(bytes, "again.dsk", &err)); + CHECK(fs.mount(dk, &err)); + } + bool find(const std::string& path, DirEntry& out) const { return fs.lookup(path, out); } +}; + +bool clean(const Fat12& fs) { + auto r = fs.verify(); + for (auto& i : r.issues) fprintf(stderr, " fsck: %s\n", i.c_str()); + return r.issues.empty(); +} + +} // namespace + +TEST(fat12_blank_720k_geometry) { + Disk d; + CHECK_EQ(d.fs.bytesPerSector(), 512); + CHECK_EQ(d.fs.sectorsPerCluster(), 2); + // 1440 - 1 boot - 2*3 FAT - 7 root = 1426 data sectors = 713 clusters. + CHECK_EQ(d.fs.totalClusters(), 713); + CHECK_EQ(d.fs.freeBytes(), 713L * 1024); + CHECK(d.fs.list("/").empty()); + CHECK(clean(d.fs)); + CHECK_EQ(d.dk.format(), DiskFormat::DSK_Plain); + CHECK_EQ(d.dk.heads(), 2); +} + +TEST(fat12_blank_360k_geometry) { + Disk d(720); + CHECK_EQ(d.fs.totalClusters(), 354); // (720 - 1 - 4 - 7) / 2 + CHECK_EQ(d.dk.heads(), 1); +} + +TEST(fat12_create_and_read_back) { + Disk d; + std::string err; + auto a = pattern(1, 1), b = pattern(1024, 2), c = pattern(5000, 3); + CHECK(d.fs.createFile("/", "A.BIN", a, &err)); + CHECK(d.fs.createFile("/", "B.BIN", b, &err)); + CHECK(d.fs.createFile("/", "c.dat", c, &err)); + CHECK(d.fs.createFile("/", "EMPTY", {}, &err)); + d.remount(); + auto ls = d.fs.list("/"); + CHECK_EQ(ls.size(), (size_t)4); + DirEntry e; + CHECK(d.find("/A.BIN", e)); CHECK(d.fs.readFile(e) == a); + CHECK(d.find("/b.bin", e)); CHECK(d.fs.readFile(e) == b); + CHECK(d.find("/C.DAT", e)); CHECK(d.fs.readFile(e) == c); CHECK_EQ(e.size, 5000u); + CHECK(d.find("/EMPTY", e)); CHECK_EQ(e.size, 0u); CHECK_EQ(e.startCluster, 0); + CHECK_EQ(e.date, 0x5A21); CHECK_EQ(e.time, 0x6000); + // 1 + 1 + 5 clusters used. + CHECK_EQ(d.fs.freeBytes(), (713L - 7) * 1024); + CHECK(clean(d.fs)); +} + +TEST(fat12_overwrite_grow_and_shrink) { + Disk d; + std::string err; + CHECK(d.fs.createFile("/", "F.BIN", pattern(100, 1), &err)); + CHECK(d.fs.createFile("/", "G.BIN", pattern(3000, 2), &err)); // fences F's chain + DirEntry e; + CHECK(d.find("/F.BIN", e)); + auto big = pattern(10000, 9); + CHECK(d.fs.writeFile(e, big, &err)); + d.remount(); + CHECK(d.find("/F.BIN", e)); CHECK(d.fs.readFile(e) == big); + CHECK(d.find("/G.BIN", e)); CHECK(d.fs.readFile(e) == pattern(3000, 2)); + CHECK_EQ(d.fs.freeBytes(), (713L - 10 - 3) * 1024); + CHECK(d.find("/F.BIN", e)); + CHECK(d.fs.writeFile(e, pattern(10, 4), &err)); + CHECK_EQ(d.fs.freeBytes(), (713L - 1 - 3) * 1024); + CHECK(d.fs.writeFile(e, {}, &err)); + CHECK_EQ(d.fs.freeBytes(), (713L - 3) * 1024); + CHECK(clean(d.fs)); +} + +TEST(fat12_delete_frees_and_lists_deleted) { + Disk d; + std::string err; + CHECK(d.fs.createFile("/", "GONE.TXT", pattern(2048, 5), &err)); + DirEntry e; + CHECK(d.find("/GONE.TXT", e)); + CHECK(d.fs.deleteFile(e, &err)); + CHECK_EQ(d.fs.freeBytes(), 713L * 1024); + CHECK(d.fs.list("/").empty()); + auto all = d.fs.list("/", true); + CHECK_EQ(all.size(), (size_t)1); + CHECK(all[0].deleted); + CHECK_EQ(all[0].name, std::string("?ONE.TXT")); + CHECK(!d.find("/GONE.TXT", e)); + CHECK(clean(d.fs)); +} + +TEST(fat12_rename) { + Disk d; + std::string err; + CHECK(d.fs.createFile("/", "OLD.BAS", pattern(10, 1), &err)); + CHECK(d.fs.createFile("/", "TAKEN.BAS", pattern(10, 2), &err)); + DirEntry e; + CHECK(d.find("/OLD.BAS", e)); + CHECK(!d.fs.renameEntry(e, "TAKEN.BAS", &err)); // duplicate + CHECK(!d.fs.renameEntry(e, "TOOLONGNAME.BAS", &err)); // not 8.3 + CHECK(!d.fs.renameEntry(e, "BAD*.BAS", &err)); // illegal char + CHECK(d.find("/OLD.BAS", e)); + CHECK(d.fs.renameEntry(e, "new.bas", &err)); + CHECK_EQ(e.name, std::string("NEW.BAS")); + d.remount(); + CHECK(d.find("/NEW.BAS", e)); + CHECK(d.fs.readFile(e) == pattern(10, 1)); + CHECK(!d.find("/OLD.BAS", e)); +} + +TEST(fat12_name_validation) { + std::string err; + CHECK(Fat12::validName83("AUTOEXEC.BAS", &err)); + CHECK(Fat12::validName83("A", &err)); + CHECK(Fat12::validName83("12345678.123", &err)); + CHECK(Fat12::validName83("GAME-1.$$$", &err)); + CHECK(!Fat12::validName83("", &err)); + CHECK(!Fat12::validName83("123456789", &err)); + CHECK(!Fat12::validName83("A.1234", &err)); + CHECK(!Fat12::validName83(".BAS", &err)); + CHECK(!Fat12::validName83("A.B.C", &err)); + CHECK(!Fat12::validName83("A B", &err)); + CHECK(!Fat12::validName83("A/B", &err)); + CHECK(!Fat12::validName83(".", &err)); +} + +TEST(fat12_create_rejects_bad_and_truncated_duplicates) { + Disk d; + std::string err; + CHECK(d.fs.createFile("/", "LONGFILE.BAS", pattern(10, 1), &err)); + // Used to be silently truncated to LONGFILE.BAS, creating a duplicate entry. + CHECK(!d.fs.createFile("/", "LONGFILENAME.BAS", pattern(10, 2), &err)); + CHECK(!d.fs.createFile("/", "longfile.bas", pattern(10, 2), &err)); + CHECK(!d.fs.createFile("/NOPE", "X.BIN", pattern(10, 2), &err)); + CHECK_EQ(d.fs.list("/").size(), (size_t)1); +} + +TEST(fat12_disk_full_rolls_back) { + Disk d(720); + std::string err; + long freeBefore = d.fs.freeBytes(); + // One byte more than the whole disk. + CHECK(!d.fs.createFile("/", "HUGE.BIN", pattern((size_t)freeBefore + 1, 1), &err)); + CHECK(!err.empty()); + CHECK_EQ(d.fs.freeBytes(), freeBefore); // no leaked clusters + CHECK(d.fs.list("/").empty()); // no phantom 0-byte entry + CHECK(clean(d.fs)); + // Growing an existing file past the end must leave it untouched. + auto small = pattern(1500, 2); + CHECK(d.fs.createFile("/", "SMALL.BIN", small, &err)); + DirEntry e; CHECK(d.find("/SMALL.BIN", e)); + long freeMid = d.fs.freeBytes(); + CHECK(!d.fs.writeFile(e, pattern((size_t)freeBefore + 10, 3), &err)); + CHECK_EQ(d.fs.freeBytes(), freeMid); + CHECK(d.find("/SMALL.BIN", e)); + CHECK(d.fs.readFile(e) == small); + // Exactly filling the disk works. + CHECK(d.fs.createFile("/", "FILL.BIN", pattern((size_t)freeMid, 4), &err)); + CHECK_EQ(d.fs.freeBytes(), 0L); + CHECK(clean(d.fs)); +} + +TEST(fat12_root_directory_full) { + Disk d; + std::string err; + for (int i = 0; i < 112; ++i) { + char nm[16]; snprintf(nm, sizeof nm, "F%03d.TXT", i); + CHECK(d.fs.createFile("/", nm, {}, &err)); + } + CHECK(!d.fs.createFile("/", "ONEMORE.TXT", {}, &err)); + CHECK_EQ(d.fs.list("/").size(), (size_t)112); +} + +TEST(fat12_subdirectories) { + Disk d; + std::string err; + CHECK(d.fs.makeDir("/", "GAMES", &err)); + CHECK(d.fs.makeDir("/GAMES", "SAVE", &err)); + CHECK(!d.fs.makeDir("/", "games", &err)); // exists + auto data = pattern(3333, 7); + CHECK(d.fs.createFile("/GAMES/SAVE", "SLOT1.DAT", data, &err)); + d.remount(); + DirEntry e; + CHECK(d.find("/GAMES", e)); CHECK(e.isDir()); + CHECK(d.find("/games/save/slot1.dat", e)); + CHECK(d.fs.readFile(e) == data); + auto sub = d.fs.list("/GAMES/SAVE"); + // ".", "..", SLOT1.DAT + CHECK_EQ(sub.size(), (size_t)3); + CHECK_EQ(sub[0].name, std::string(".")); + CHECK_EQ(sub[1].name, std::string("..")); + CHECK(clean(d.fs)); +} + +TEST(fat12_subdirectory_grows_past_one_cluster) { + Disk d; + std::string err; + CHECK(d.fs.makeDir("/", "MANY", &err)); + // One 1KB cluster holds 32 entries; "." and ".." take two. + for (int i = 0; i < 100; ++i) { + char nm[16]; snprintf(nm, sizeof nm, "N%03d", i); + CHECK(d.fs.createFile("/MANY", nm, pattern(1, (uint8_t)i), &err)); + } + d.remount(); + CHECK_EQ(d.fs.list("/MANY").size(), (size_t)102); + DirEntry e; + CHECK(d.find("/MANY/N099", e)); + CHECK(d.fs.readFile(e) == pattern(1, 99)); + CHECK(clean(d.fs)); +} + +TEST(fat12_remove_empty_directory) { + Disk d; + std::string err; + CHECK(d.fs.makeDir("/", "TMP", &err)); + CHECK(d.fs.createFile("/TMP", "X", pattern(5, 1), &err)); + DirEntry dir; + CHECK(d.find("/TMP", dir)); + CHECK(!d.fs.removeDir(dir, &err)); // not empty + DirEntry x; CHECK(d.find("/TMP/X", x)); + CHECK(d.fs.deleteFile(x, &err)); + CHECK(d.find("/TMP", dir)); + CHECK(d.fs.removeDir(dir, &err)); + CHECK(!d.find("/TMP", dir)); + CHECK_EQ(d.fs.freeBytes(), 713L * 1024); + CHECK(clean(d.fs)); +} + +TEST(fat12_fsck_detects_corruption) { + Disk d; + std::string err; + CHECK(d.fs.createFile("/", "A.BIN", pattern(3000, 1), &err)); + CHECK(d.fs.createFile("/", "B.BIN", pattern(3000, 2), &err)); + DirEntry a, b; + CHECK(d.find("/A.BIN", a)); CHECK(d.find("/B.BIN", b)); + // Point B's directory entry at A's chain: a cross-link. + auto& raw = d.dk.rawMut(); + raw[(size_t)b.entryOffset + 0x1A] = (uint8_t)(a.startCluster & 0xFF); + raw[(size_t)b.entryOffset + 0x1B] = (uint8_t)(a.startCluster >> 8); + auto r = d.fs.verify(); + bool cross = false, lost = false; + for (auto& i : r.issues) { + if (i.find("cross-linked") != std::string::npos) cross = true; + } + CHECK(cross); + CHECK_EQ(r.unreferenced, 3L); // B's original 3 clusters are now orphaned + (void)lost; +} + +TEST(fat12_fsck_detects_size_mismatch) { + Disk d; + std::string err; + CHECK(d.fs.createFile("/", "A.BIN", pattern(3000, 1), &err)); // 3 clusters + DirEntry a; CHECK(d.find("/A.BIN", a)); + auto& raw = d.dk.rawMut(); + raw[(size_t)a.entryOffset + 0x1C] = 0x00; // size 3000 -> 0x0B00 & 0xFF00 = 2816 + raw[(size_t)a.entryOffset + 0x1D] = 0x30; // 0x3000 = 12288 bytes > 3 clusters + auto r = d.fs.verify(); + bool found = false; + for (auto& i : r.issues) if (i.find("size") != std::string::npos) found = true; + CHECK(found); +} + +TEST(fat12_self_referencing_directory_terminates) { + Disk d; + std::string err; + CHECK(d.fs.makeDir("/", "LOOP", &err)); + DirEntry loop; CHECK(d.find("/LOOP", loop)); + // Plant an entry inside LOOP that points back at LOOP itself. + CHECK(d.fs.makeDir("/LOOP", "BACK", &err)); + DirEntry back; CHECK(d.find("/LOOP/BACK", back)); + auto& raw = d.dk.rawMut(); + raw[(size_t)back.entryOffset + 0x1A] = (uint8_t)(loop.startCluster & 0xFF); + raw[(size_t)back.entryOffset + 0x1B] = (uint8_t)(loop.startCluster >> 8); + auto r = d.fs.verify(); // must return, not recurse forever + CHECK(!r.issues.empty()); + auto map = d.fs.clusterMap(); // same + CHECK(!map.empty()); +} + +TEST(fat12_raw_unnamed_roundtrip) { + Disk d; + std::string err; + CHECK(d.fs.createFile("/", "LOADER.BIN", pattern(700, 1), &err)); + CHECK(d.fs.createFile("/", "PAYLOAD", pattern(4096, 2), &err)); + DirEntry p; CHECK(d.find("/PAYLOAD", p)); + // Turn PAYLOAD into "RAW" data: wipe its directory entry but keep the FAT chain, + // like loader disks do. + auto& raw = d.dk.rawMut(); + memset(&raw[(size_t)p.entryOffset], 0, 32); + // Put LOADER back at the start of the table so the directory stays contiguous. + CHECK_EQ(d.fs.rawUnnamedCapacity(), 4096L); + auto blob = d.fs.extractRawUnnamed(); + CHECK(blob == pattern(4096, 2)); + auto repl = pattern(4096, 3); + long written = 0; + CHECK(d.fs.writeRawUnnamed(repl, &written, &err)); + CHECK_EQ(written, 4096L); + CHECK(d.fs.extractRawUnnamed() == repl); + CHECK(!d.fs.writeRawUnnamed(pattern(5000, 4), &written, &err)); + CHECK_EQ(written, 4096L); +} + +TEST(fat12_mount_rejects_garbage) { + DiskImage dk; Fat12 fs; std::string err; + std::vector junk(737280, 0x00); + CHECK(dk.loadBytes(junk, "zero.dsk", &err)); + CHECK(!fs.mount(dk, &err)); +} + +TEST(fat12_mount_falls_back_to_media_byte) { + // MSX-DOS 1 era disks may carry a boot sector without a usable BPB; the + // geometry then comes from the media descriptor in the first FAT byte. + std::string err; + auto img = formatBlankImage(1440); + { + DiskImage dk; Fat12 fs; + CHECK(dk.loadBytes(img, "a.dsk", &err)); CHECK(fs.mount(dk, &err)); + CHECK(fs.createFile("/", "HELLO.TXT", pattern(1234, 1), &err)); + img = dk.raw(); + } + memset(img.data() + 3, 0, 0x1E - 3); // wipe the BPB (keep the FAT/root) + DiskImage dk; Fat12 fs; + CHECK(dk.loadBytes(img, "b.dsk", &err)); + CHECK(fs.mount(dk, &err)); + CHECK_EQ(fs.totalClusters(), 713); + DirEntry e; CHECK(fs.lookup("/HELLO.TXT", e)); + CHECK(fs.readFile(e) == pattern(1234, 1)); +} + +TEST(fat12_dir_entry_strings) { + DirEntry e; + e.date = (uint16_t)(((2024 - 1980) << 9) | (3 << 5) | 15); + e.time = (uint16_t)((13 << 11) | (45 << 5) | (30 / 2)); + e.attr = 0x21; + CHECK_EQ(e.dateString(), std::string("2024-03-15")); + CHECK_EQ(e.timeString(), std::string("13:45:30")); + CHECK_EQ(e.attrString(), std::string("R----A")); +} diff --git a/tests/test_main.cpp b/tests/test_main.cpp new file mode 100644 index 0000000..243d95e --- /dev/null +++ b/tests/test_main.cpp @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (C) 2026 Alonso (GS·RUN). MSXFileForge. +// Free software under the GNU GPL v3 (or later) - see LICENSE. Modified +// versions must be distributed under the same license. +// Test runner: `msxff_tests` runs everything, `msxff_tests ` filters. +#include "check.h" +#include + +namespace msxtest { +std::vector& registry() { static std::vector r; return r; } +static int g_failures = 0; +void fail(const char* file, int line, const std::string& what) { + fprintf(stderr, " %s:%d: %s\n", file, line, what.c_str()); + g_failures++; +} +} // namespace msxtest + +int main(int argc, char** argv) { + using namespace msxtest; + int run = 0, bad = 0; + for (auto& c : registry()) { + if (argc > 1 && !strstr(c.name, argv[1])) continue; + int before = g_failures; + c.fn(); + run++; + bool ok = g_failures == before; + if (!ok) bad++; + printf("%s %s\n", ok ? "[ ok ]" : "[FAIL]", c.name); + } + printf("\n%d test(s), %d failed\n", run, bad); + return (bad || run == 0) ? 1 : 0; +} diff --git a/tests/test_rom.cpp b/tests/test_rom.cpp new file mode 100644 index 0000000..f106c1d --- /dev/null +++ b/tests/test_rom.cpp @@ -0,0 +1 @@ +#include "check.h" diff --git a/tests/test_tape.cpp b/tests/test_tape.cpp new file mode 100644 index 0000000..f106c1d --- /dev/null +++ b/tests/test_tape.cpp @@ -0,0 +1 @@ +#include "check.h" From 08192a667e0f1fc5861ed56b4036d357fcbb6b4e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 13:15:18 +0000 Subject: [PATCH 2/8] basic: crunch programs the way the MSX-BASIC ROM does Tokenizer fixes (each covered by tests/test_basic.cpp): - "&B1010" (and any '&' not followed by H/O/octal) hung forever. - Digits inside variable names (A1, X12$) were crunched to numeric constants, which MSX-BASIC rejects as a syntax error; they stay ASCII. - DATA items are stored verbatim up to ':' (they were crunched). - ELSE is stored as ':' ELSE and ' as ':' REM ''', as on a real MSX; the detokenizer hides the colon again. - Names are upper-cased outside strings, REM and DATA. - '!' / 'E' give single precision (0x1D), '#' / 'D' / reals double (0x1F); an E/D only starts an exponent when digits follow ("1ELSE"). - RETURN and RESUME take line numbers; LIST/DELETE ranges keep both. - Lines are sorted; a repeated number replaces, a bare number deletes. - BCD is built from the decimal digits with half-up rounding instead of binary floating point, so constants are exact. The detokenizer formats reals from the BCD digits (".5", "1.5!", "1#", "1D+20", "2.5E-10"), so detokenize -> tokenize reproduces the bytes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CtBtjm9PRm6sDhDVJbtciC --- core/msxfile.cpp | 348 ++++++++++++++++++++++++++++++------------- tests/CMakeLists.txt | 1 + tests/test_basic.cpp | 227 ++++++++++++++++++++++++++++ 3 files changed, 469 insertions(+), 107 deletions(-) diff --git a/core/msxfile.cpp b/core/msxfile.cpp index d36b91c..b4436d6 100644 --- a/core/msxfile.cpp +++ b/core/msxfile.cpp @@ -90,30 +90,99 @@ static const char* extTokenText(uint8_t b) { return nullptr; } -// Decode a Microsoft/MSX BCD float. mant = number of mantissa bytes (3=single, -// 7=double). Returns the value as a double; caller formats it. -static double decodeBcd(const uint8_t* p, int mantBytes) { - uint8_t expByte = p[0]; - if (expByte == 0) return 0.0; - bool neg = (expByte & 0x80) != 0; - int exp = (expByte & 0x7F) - 0x40; - double mant = 0.0, scale = 0.1; +// ---- Numeric constants ---------------------------------------------------- +// +// MSX-BASIC stores real constants as BCD: one exponent byte (bit 7 = sign, +// bits 0-6 = power of ten + 0x40, value = 0.d1d2d3... x 10^exp) followed by 3 +// (single precision, 6 digits, token 0x1D) or 7 (double, 14 digits, token +// 0x1F) mantissa bytes, two digits per byte. Conversion goes through decimal +// digit strings, never binary floating point, so it is exact both ways. + +namespace { + +struct DecNum { // value = 0.digits x 10^exp ("" == zero) + std::string digits; // no leading/trailing zeros + int exp = 0; + bool neg = false; +}; + +DecNum bcdToDec(const uint8_t* p, int mantBytes) { + DecNum n; + if (p[0] == 0) return n; + n.neg = (p[0] & 0x80) != 0; + n.exp = (p[0] & 0x7F) - 0x40; for (int i = 1; i <= mantBytes; ++i) { - int hi = (p[i] >> 4) & 0xF, lo = p[i] & 0xF; - mant += hi * scale; scale *= 0.1; - mant += lo * scale; scale *= 0.1; + n.digits += (char)('0' + ((p[i] >> 4) % 10)); + n.digits += (char)('0' + ((p[i] & 0xF) % 10)); + } + size_t lead = n.digits.find_first_not_of('0'); + if (lead == std::string::npos) return DecNum{}; + n.exp -= (int)lead; + n.digits.erase(0, lead); + while (!n.digits.empty() && n.digits.back() == '0') n.digits.pop_back(); + return n; +} + +// Round to `maxDigits` significant digits (half up) and pack into p[0..mant]. +void decToBcd(DecNum n, int mantBytes, uint8_t* p) { + int maxDigits = mantBytes * 2; + memset(p, 0, (size_t)mantBytes + 1); + if (n.digits.empty()) return; + if ((int)n.digits.size() > maxDigits) { + bool up = n.digits[(size_t)maxDigits] >= '5'; + n.digits.resize((size_t)maxDigits); + for (int i = maxDigits - 1; up && i >= 0; --i) { + if (n.digits[(size_t)i] == '9') n.digits[(size_t)i] = '0'; + else { n.digits[(size_t)i]++; up = false; } + } + if (up) { n.digits.insert(n.digits.begin(), '1'); n.digits.pop_back(); n.exp++; } } - double v = mant * std::pow(10.0, exp); - return neg ? -v : v; + if (n.exp > 63) { n.exp = 63; n.digits = std::string((size_t)maxDigits, '9'); } // overflow: clamp + if (n.exp < -63) return; // underflow: zero + n.digits.resize((size_t)maxDigits, '0'); + p[0] = (uint8_t)((n.exp + 0x40) | (n.neg ? 0x80 : 0)); + for (int i = 0; i < mantBytes; ++i) + p[i + 1] = (uint8_t)(((n.digits[(size_t)i * 2] - '0') << 4) | (n.digits[(size_t)i * 2 + 1] - '0')); } -static void appendNumber(std::string& out, double v, bool integral) { - char buf[64]; - if (integral) { snprintf(buf, sizeof buf, "%.0f", v); out += buf; return; } - snprintf(buf, sizeof buf, "%.9g", v); - out += buf; +// Format the way a listing should show it so that re-tokenizing gives the same +// bytes: no leading zero (".5"), single precision marked with '!' (or an 'E' +// exponent), double precision with a 'D' exponent, or '#' when it would +// otherwise read back as an integer. +std::string formatReal(const DecNum& n, bool dbl) { + const int maxDigits = dbl ? 14 : 6; + std::string s; + if (n.neg) s += '-'; + if (n.digits.empty()) return s + (dbl ? "0#" : "0!"); + const std::string& d = n.digits; + int len = (int)d.size(); + bool fixed = (n.exp >= 1 && n.exp <= maxDigits) || + (n.exp <= 0 && -n.exp + len <= maxDigits && n.exp >= -(maxDigits - 1)); + if (fixed) { + if (n.exp <= 0) { + s += '.'; s.append((size_t)-n.exp, '0'); s += d; + } else if (len <= n.exp) { + s += d; s.append((size_t)(n.exp - len), '0'); + } else { + s += d.substr(0, (size_t)n.exp); s += '.'; s += d.substr((size_t)n.exp); + } + bool hasPoint = s.find('.') != std::string::npos; + if (!dbl) s += '!'; + else if (!hasPoint) { // would it read back as an integer? + std::string mag = n.neg ? s.substr(1) : s; + if (mag.size() < 5 || (mag.size() == 5 && mag <= "32767")) s += '#'; + } + return s; + } + s += d[0]; + if (len > 1) { s += '.'; s += d.substr(1); } + int e = n.exp - 1; + char buf[16]; snprintf(buf, sizeof buf, "%c%c%02d", dbl ? 'D' : 'E', e < 0 ? '-' : '+', e < 0 ? -e : e); + return s + buf; } +} // namespace + std::string detokenizeBasic(const std::vector& d) { std::string out; if (d.empty() || d[0] != 0xFF) return out; // not tokenized @@ -126,7 +195,7 @@ std::string detokenizeBasic(const std::vector& d) { char num[16]; snprintf(num, sizeof num, "%u ", lineNo); out += num; - bool inString = false, restLiteral = false; + bool inString = false, restLiteral = false, inData = false; while (pos < d.size()) { uint8_t b = d[pos]; if (b == 0x00) { pos++; break; } // end of line @@ -138,6 +207,16 @@ std::string detokenizeBasic(const std::vector& d) { continue; } if (b == '"') { inString = true; out += '"'; pos++; continue; } + if (inData) { // DATA items are stored verbatim + if (b == ':') inData = false; + out += (char)b; pos++; continue; + } + + // ELSE is stored as ":" ELSE and ' as ":" REM "'"; LIST hides the colon. + if (b == ':' && pos + 1 < d.size() && d[pos+1] == 0xA1) { pos++; continue; } + if (b == ':' && pos + 2 < d.size() && d[pos+1] == 0x8F && d[pos+2] == 0xE6) { + out += '\''; pos += 3; restLiteral = true; continue; + } // Embedded numeric constants. if (b == 0x0B && pos + 2 < d.size()) { // octal @@ -162,11 +241,11 @@ std::string detokenizeBasic(const std::vector& d) { int16_t v = (int16_t)(d[pos+1] | (d[pos+2] << 8)); out += std::to_string((int)v); pos += 3; continue; } - if (b == 0x1D && pos + 4 < d.size()) { // single float (BCD) - appendNumber(out, decodeBcd(&d[pos+1], 3), false); pos += 5; continue; + if (b == 0x1D && pos + 4 < d.size()) { // single precision (BCD) + out += formatReal(bcdToDec(&d[pos+1], 3), false); pos += 5; continue; } - if (b == 0x1F && pos + 8 < d.size()) { // double float (BCD) - appendNumber(out, decodeBcd(&d[pos+1], 7), false); pos += 9; continue; + if (b == 0x1F && pos + 8 < d.size()) { // double precision (BCD) + out += formatReal(bcdToDec(&d[pos+1], 7), true); pos += 9; continue; } // Keyword tokens. @@ -180,6 +259,7 @@ std::string detokenizeBasic(const std::vector& d) { if (kw) { out += kw; if (b == 0x8F || b == 0xE6) restLiteral = true; // REM / ' + if (b == 0x84) inData = true; // DATA pos++; continue; } @@ -190,7 +270,6 @@ std::string detokenizeBasic(const std::vector& d) { pos++; } out += '\n'; - pos = pos; // link is advisory; we stream sequentially } return out; } @@ -292,7 +371,7 @@ const std::vector& keywordList() { if (t) v.push_back({t, (uint8_t)b, true}); } v.push_back({"?", 0x91, false}); // ? is PRINT - std::sort(v.begin(), v.end(), [](const Kw& a, const Kw& b) { + std::stable_sort(v.begin(), v.end(), [](const Kw& a, const Kw& b) { return a.text.size() > b.text.size(); }); return v; @@ -300,32 +379,78 @@ const std::vector& keywordList() { return kws; } -// Encode a Microsoft/MSX BCD double (7 mantissa bytes) into out[8]. -void encodeBcdDouble(double v, uint8_t out[8]) { - memset(out, 0, 8); - if (v == 0.0) return; - bool neg = v < 0; if (neg) v = -v; - int exp = (int)std::floor(std::log10(v)) + 1; // v = mant * 10^exp, mant in [0.1,1) - double mant = v / std::pow(10.0, exp); - // Guard rounding that can push mant to 1.0 - if (mant >= 1.0) { mant /= 10.0; exp++; } - out[0] = (uint8_t)((exp + 0x40) | (neg ? 0x80 : 0)); - for (int i = 1; i <= 7; ++i) { - int hi = (int)(mant * 10); mant = mant * 10 - hi; - int lo = (int)(mant * 10); mant = mant * 10 - lo; - out[i] = (uint8_t)((hi << 4) | lo); - } -} - bool isJumpToken(uint8_t code) { // integers after these are line numbers switch (code) { case 0x89: case 0x8D: case 0xDA: case 0xA1: // GOTO GOSUB THEN ELSE case 0x8C: case 0x8A: case 0xA9: case 0xAA: // RESTORE RUN AUTO RENUM - case 0x93: case 0xA8: // LIST DELETE + case 0x93: case 0xA8: case 0xA7: case 0x8E: // LIST DELETE RESUME RETURN return true; } return false; } + +bool isDigit(char c) { return c >= '0' && c <= '9'; } +bool isAlpha(char c) { return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'); } + +// Parse a decimal number at ln[i]: digits, optional '.', optional E/D +// exponent (only when digits follow), optional type suffix ! # %. +struct ParsedNum { + DecNum dec; + bool hasPoint = false; + char expChar = 0; // 'E' or 'D' if an exponent was given + char suffix = 0; // '!' '#' '%' or 0 + bool integral = false; // no point, no exponent + double approx = 0; // only for range checks +}; + +size_t parseNumber(const std::string& ln, size_t i, ParsedNum& n) { + size_t m = ln.size(); + std::string digits; + int pointPos = -1; + while (i < m && (isDigit(ln[i]) || (ln[i] == '.' && pointPos < 0))) { + if (ln[i] == '.') pointPos = (int)digits.size(); + else digits += ln[i]; + i++; + } + n.hasPoint = pointPos >= 0; + if (pointPos < 0) pointPos = (int)digits.size(); + int exp10 = 0; + if (i < m && (ln[i] == 'E' || ln[i] == 'e' || ln[i] == 'D' || ln[i] == 'd')) { + size_t j = i + 1; + bool negE = false; + if (j < m && (ln[j] == '+' || ln[j] == '-')) { negE = ln[j] == '-'; j++; } + if (j < m && isDigit(ln[j])) { + n.expChar = (char)toupper((unsigned char)ln[i]); + int e = 0; + while (j < m && isDigit(ln[j])) { if (e < 1000) e = e * 10 + (ln[j] - '0'); j++; } + exp10 = negE ? -e : e; + i = j; + } + } + if (i < m && (ln[i] == '!' || ln[i] == '#' || ln[i] == '%')) n.suffix = ln[i++]; + n.integral = !n.hasPoint && !n.expChar; + + size_t lead = digits.find_first_not_of('0'); + if (lead == std::string::npos) { n.dec = DecNum{}; n.approx = 0; return i; } + n.dec.exp = pointPos - (int)lead + exp10; + n.dec.digits = digits.substr(lead); + while (!n.dec.digits.empty() && n.dec.digits.back() == '0') n.dec.digits.pop_back(); + n.approx = atof(("0." + n.dec.digits).c_str()) * std::pow(10.0, n.dec.exp); + return i; +} + +void emitReal(std::vector& body, const DecNum& dec, bool dbl) { + uint8_t b[8]; + decToBcd(dec, dbl ? 7 : 3, b); + body.push_back(dbl ? 0x1F : 0x1D); + body.insert(body.end(), b, b + (dbl ? 8 : 4)); +} + +void emitInt(std::vector& body, long v) { + if (v >= 0 && v <= 9) body.push_back((uint8_t)(0x11 + v)); + else if (v >= 10 && v <= 255) { body.push_back(0x0F); body.push_back((uint8_t)v); } + else { body.push_back(0x1C); body.push_back(v & 0xFF); body.push_back((v >> 8) & 0xFF); } +} } // namespace std::vector tokenizeBasic(const std::string& text) { @@ -341,115 +466,124 @@ std::vector tokenizeBasic(const std::string& text) { size_t eol = text.find('\n', p); std::string ln = text.substr(p, (eol == std::string::npos ? n : eol) - p); p = (eol == std::string::npos) ? n : eol + 1; - // strip trailing CR if (!ln.empty() && ln.back() == '\r') ln.pop_back(); size_t i = 0, m = ln.size(); while (i < m && ln[i] == ' ') i++; - if (i >= m) continue; // blank line - if (!isdigit((unsigned char)ln[i])) continue; // no line number -> skip + if (i >= m) continue; // blank line + if (!isDigit(ln[i])) continue; // no line number -> skip long no = 0; - while (i < m && isdigit((unsigned char)ln[i])) no = no * 10 + (ln[i++] - '0'); - while (i < m && ln[i] == ' ') i++; + while (i < m && isDigit(ln[i])) { no = no * 10 + (ln[i++] - '0'); if (no > 99999) no = 99999; } + if (no > 65529) continue; // not a valid MSX line number + if (i < m && ln[i] == ' ') i++; // the separator LIST prints Line L; L.no = (uint16_t)no; - bool lineRef = false; + bool lineRef = false; // integers are line numbers (after GOTO etc.) + bool inIdent = false; // inside a variable name: digits stay ASCII while (i < m) { char c = ln[i]; - // String literal + // String literal (stored verbatim, closing quote optional). if (c == '"') { L.body.push_back('"'); i++; while (i < m && ln[i] != '"') L.body.push_back((uint8_t)ln[i++]); if (i < m) { L.body.push_back('"'); i++; } - lineRef = false; + lineRef = inIdent = false; continue; } - // Keyword / operator match (longest first, case-insensitive) + // Digits that continue a variable name (A1, X12$) are plain ASCII. + if (inIdent && isDigit(c)) { L.body.push_back((uint8_t)c); i++; continue; } + + // Keyword / operator match (longest first, case-insensitive). const Kw* best = nullptr; for (const auto& kw : keywordList()) { size_t L2 = kw.text.size(); if (i + L2 > m) continue; bool eq = true; for (size_t k = 0; k < L2; ++k) { - char a = ln[i + k], b = kw.text[k]; - if (toupper((unsigned char)a) != toupper((unsigned char)b)) { eq = false; break; } + if (toupper((unsigned char)ln[i + k]) != (unsigned char)kw.text[k]) { eq = false; break; } } if (eq) { best = &kw; break; } // list is longest-first } if (best) { + i += best->text.size(); + inIdent = false; + if (best->code == 0xE6 && !best->ext) { // ' -> : REM ' + L.body.push_back(':'); L.body.push_back(0x8F); L.body.push_back(0xE6); + while (i < m) L.body.push_back((uint8_t)ln[i++]); + break; + } + if (best->code == 0xA1 && !best->ext) L.body.push_back(':'); // ELSE -> : ELSE if (best->ext) { L.body.push_back(0xFF); L.body.push_back(best->code); } else L.body.push_back(best->code); - i += best->text.size(); - if (best->code == 0x8F || best->code == 0xE6) { // REM / ' + if (!best->ext && best->code == 0x8F) { // REM: rest is literal while (i < m) L.body.push_back((uint8_t)ln[i++]); break; } - lineRef = isJumpToken(best->code) || (lineRef && best->code == 0x8A); + if (!best->ext && best->code == 0x84) { // DATA: literal up to ':' + bool q = false; + while (i < m && (q || ln[i] != ':')) { + if (ln[i] == '"') q = !q; + L.body.push_back((uint8_t)ln[i++]); + } + lineRef = false; + continue; + } + bool keepRange = lineRef && !best->ext && best->code == 0xF2; // LIST 10-20 + lineRef = (!best->ext && isJumpToken(best->code)) || keepRange; continue; } - // Number literal - if (isdigit((unsigned char)c) || (c == '&') || - (c == '.' && i + 1 < m && isdigit((unsigned char)ln[i+1]))) { - if (c == '&' && i + 1 < m && (ln[i+1] == 'H' || ln[i+1] == 'h')) { - i += 2; unsigned v = 0; - while (i < m && isxdigit((unsigned char)ln[i])) { - char d = ln[i++]; - v = v * 16 + (isdigit((unsigned char)d) ? d - '0' - : (toupper((unsigned char)d) - 'A' + 10)); + // &H hex / &O or & octal constants; &B binary stays ASCII. + if (c == '&' && i + 1 < m) { + char t = (char)toupper((unsigned char)ln[i+1]); + if (t == 'H' || t == 'O' || (ln[i+1] >= '0' && ln[i+1] <= '7')) { + bool hex = t == 'H'; + i += (t == 'H' || t == 'O') ? 2 : 1; + unsigned v = 0; + while (i < m && (hex ? isxdigit((unsigned char)ln[i]) != 0 : (ln[i] >= '0' && ln[i] <= '7'))) { + char dch = (char)toupper((unsigned char)ln[i++]); + v = v * (hex ? 16 : 8) + (unsigned)(isDigit(dch) ? dch - '0' : dch - 'A' + 10); } - L.body.push_back(0x0C); + L.body.push_back(hex ? 0x0C : 0x0B); L.body.push_back(v & 0xFF); L.body.push_back((v >> 8) & 0xFF); + inIdent = lineRef = false; continue; } - if (c == '&' && i + 1 < m && (ln[i+1] == 'O' || ln[i+1] == 'o')) { - i += 2; unsigned v = 0; - while (i < m && ln[i] >= '0' && ln[i] <= '7') v = v * 8 + (ln[i++] - '0'); - L.body.push_back(0x0B); + } + // Decimal number. + if (isDigit(c) || (c == '.' && i + 1 < m && isDigit(ln[i+1]))) { + ParsedNum pn; + size_t end = parseNumber(ln, i, pn); + if (lineRef && pn.integral && !pn.suffix && pn.approx <= 65535) { + long v = atol(ln.substr(i, end - i).c_str()); + L.body.push_back(0x0E); L.body.push_back(v & 0xFF); L.body.push_back((v >> 8) & 0xFF); - continue; - } - // Decimal integer or float - size_t s = i; bool isFloat = false; - while (i < m && isdigit((unsigned char)ln[i])) i++; - if (i < m && ln[i] == '.') { isFloat = true; i++; while (i < m && isdigit((unsigned char)ln[i])) i++; } - if (i < m && (ln[i] == 'E' || ln[i] == 'e' || ln[i] == 'D' || ln[i] == 'd')) { - isFloat = true; i++; - if (i < m && (ln[i] == '+' || ln[i] == '-')) i++; - while (i < m && isdigit((unsigned char)ln[i])) i++; - } - std::string num = ln.substr(s, i - s); - if (isFloat) { - double v = atof(num.c_str()); - uint8_t b[8]; encodeBcdDouble(v, b); - L.body.push_back(0x1F); - for (int k = 0; k < 8; ++k) L.body.push_back(b[k]); + } else if (pn.suffix == '!' || (pn.expChar == 'E' && pn.suffix != '#')) { + emitReal(L.body, pn.dec, false); + } else if (pn.suffix == '#' || pn.expChar == 'D' || !pn.integral || pn.approx > 32767) { + emitReal(L.body, pn.dec, true); } else { - long v = atol(num.c_str()); - if (lineRef) { - L.body.push_back(0x0E); - L.body.push_back(v & 0xFF); L.body.push_back((v >> 8) & 0xFF); - } else if (v >= 0 && v <= 9) { - L.body.push_back((uint8_t)(0x11 + v)); - } else if (v >= 10 && v <= 255) { - L.body.push_back(0x0F); L.body.push_back((uint8_t)v); - } else if (v >= -32768 && v <= 32767) { - L.body.push_back(0x1C); - L.body.push_back(v & 0xFF); L.body.push_back((v >> 8) & 0xFF); - } else { - uint8_t b[8]; encodeBcdDouble((double)v, b); - L.body.push_back(0x1F); - for (int k = 0; k < 8; ++k) L.body.push_back(b[k]); - } + emitInt(L.body, atol(ln.substr(i, end - i).c_str())); + if (pn.suffix == '%') L.body.push_back('%'); } + i = end; + inIdent = false; continue; } - // Literal character. Keep line-ref mode only across spaces and - // commas (for ON..GOTO n,n,n lists); anything else ends it. + // Literal character. Letters start/continue a variable name and are + // stored in upper case, as the MSX-BASIC cruncher does. + if (isAlpha(c)) { inIdent = true; c = (char)toupper((unsigned char)c); } + else inIdent = false; + // Keep line-ref mode only across spaces and commas (ON..GOTO lists). if (c != ' ' && c != ',') lineRef = false; L.body.push_back((uint8_t)c); i++; } - lines.push_back(std::move(L)); + // A later line with the same number replaces the earlier one; a bare + // line number deletes it (as typing it at the prompt would). + auto it = std::find_if(lines.begin(), lines.end(), [&](const Line& x) { return x.no == L.no; }); + if (it != lines.end()) lines.erase(it); + if (!L.body.empty()) lines.push_back(std::move(L)); } + std::stable_sort(lines.begin(), lines.end(), [](const Line& a, const Line& b) { return a.no < b.no; }); // Emit with computed link addresses (base 0x8001). uint16_t addr = 0x8001; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b5f8536..95f1a8d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,3 +2,4 @@ add_executable(msxff_tests test_main.cpp test_fat12.cpp test_basic.cpp test_tape.cpp test_rom.cpp test_disk.cpp) target_link_libraries(msxff_tests PRIVATE msxcore) add_test(NAME core COMMAND msxff_tests) +set_tests_properties(core PROPERTIES TIMEOUT 120) diff --git a/tests/test_basic.cpp b/tests/test_basic.cpp index f106c1d..f82e93e 100644 --- a/tests/test_basic.cpp +++ b/tests/test_basic.cpp @@ -1 +1,228 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (C) 2026 Alonso (GS·RUN). MSXFileForge. +// Free software under the GNU GPL v3 (or later) - see LICENSE. Modified +// versions must be distributed under the same license. +// MSX-BASIC tokenizer / detokenizer, file type detection and block carving. #include "check.h" +#include "msxfile.h" + +using namespace msx; + +namespace { + +typedef std::vector Bytes; + +// Body bytes of the first (only) line of a tokenized program. +Bytes body(const std::string& line) { + Bytes t = tokenizeBasic(line); + // FF, link(2), lineno(2), body..., 00, 00 00 + if (t.size() < 8) return {}; + return Bytes(t.begin() + 5, t.end() - 3); +} + +std::string hex(const Bytes& b) { + std::string s; char t[4]; + for (auto x : b) { snprintf(t, sizeof t, "%02X ", x); s += t; } + if (!s.empty()) s.pop_back(); + return s; +} + +#define CHECK_BODY(src, expected) CHECK_EQ(hex(body(src)), std::string(expected)) + +} // namespace + +TEST(basic_program_layout) { + Bytes t = tokenizeBasic("10 PRINT 1\n20 END\n"); + // Line 10: link 0x8009 (0x8001 + 4 header + 3 body + 1 terminator). + CHECK_EQ(hex(t), std::string("FF 09 80 0A 00 91 20 12 00 0F 80 14 00 81 00 00 00")); +} + +TEST(basic_keywords_and_case) { + CHECK_BODY("10 print a", "91 20 41"); + CHECK_BODY("10 ? \"hi\"", "91 20 22 68 69 22"); + CHECK_BODY("10 X=LEFT$(A$,2)", "58 EF FF 81 28 41 24 2C 13 29"); + CHECK_BODY("10 IF A>=B THEN STOP", "8B 20 41 EE EF 42 20 DA 20 90"); +} + +TEST(basic_variable_digits_stay_ascii) { + // Regression: "A1" was crunched to 'A' + constant 1, a syntax error on MSX. + CHECK_BODY("10 A1=5", "41 31 EF 16"); + CHECK_BODY("10 X12$=\"\"", "58 31 32 24 EF 22 22"); + CHECK_BODY("10 A=B2+3", "41 EF 42 32 F1 14"); +} + +TEST(basic_integer_constants) { + CHECK_BODY("10 A=0", "41 EF 11"); + CHECK_BODY("10 A=9", "41 EF 1A"); + CHECK_BODY("10 A=10", "41 EF 0F 0A"); + CHECK_BODY("10 A=255", "41 EF 0F FF"); + CHECK_BODY("10 A=256", "41 EF 1C 00 01"); + CHECK_BODY("10 A=32767", "41 EF 1C FF 7F"); + // Beyond the integer range: double precision. + CHECK_BODY("10 A=32768", "41 EF 1F 45 32 76 80 00 00 00 00"); +} + +TEST(basic_real_constants) { + CHECK_BODY("10 A=1.5", "41 EF 1F 41 15 00 00 00 00 00 00"); + CHECK_BODY("10 A=1.5!", "41 EF 1D 41 15 00 00"); + CHECK_BODY("10 A=1#", "41 EF 1F 41 10 00 00 00 00 00 00"); + CHECK_BODY("10 A=.01", "41 EF 1F 3F 10 00 00 00 00 00 00"); + CHECK_BODY("10 A=0.1", "41 EF 1F 40 10 00 00 00 00 00 00"); + CHECK_BODY("10 A=1E5", "41 EF 1D 46 10 00 00"); + CHECK_BODY("10 A=1D-3", "41 EF 1F 3E 10 00 00 00 00 00 00"); + CHECK_BODY("10 A=3.14159265358979", "41 EF 1F 41 31 41 59 26 53 58 98"); + // 15 significant digits round half-up to 14. + CHECK_BODY("10 A=123456789012345", "41 EF 1F 4F 12 34 56 78 90 12 35"); + CHECK_BODY("10 A=9.9999999999999", "41 EF 1F 41 99 99 99 99 99 99 99"); + CHECK_BODY("10 A=9.999999999999999", "41 EF 1F 42 10 00 00 00 00 00 00"); + CHECK_BODY("10 A=1.2345678!", "41 EF 1D 41 12 34 57"); +} + +TEST(basic_exponent_needs_digits) { + // "1ELSE": the E belongs to ELSE, not to an exponent. + CHECK_BODY("10 IF A THEN 1ELSE 2", "8B 20 41 20 DA 20 0E 01 00 3A A1 20 0E 02 00"); +} + +TEST(basic_hex_octal_binary) { + CHECK_BODY("10 A=&HFF", "41 EF 0C FF 00"); + CHECK_BODY("10 A=&hc000", "41 EF 0C 00 C0"); + CHECK_BODY("10 A=&O17", "41 EF 0B 0F 00"); + CHECK_BODY("10 A=&17", "41 EF 0B 0F 00"); + // &B is not crunched: stays ASCII. Used to hang the tokenizer forever. + CHECK_BODY("10 A=&B101", "41 EF 26 42 31 30 31"); + CHECK_BODY("10 A$=\"&\"+B$", "41 24 EF 22 26 22 F1 42 24"); +} + +TEST(basic_line_references) { + CHECK_BODY("10 GOTO 100", "89 20 0E 64 00"); + CHECK_BODY("10 GOSUB 5", "8D 20 0E 05 00"); + CHECK_BODY("10 ON X GOTO 10,20,30", "95 20 58 20 89 20 0E 0A 00 2C 0E 14 00 2C 0E 1E 00"); + CHECK_BODY("10 RESTORE 1000", "8C 20 0E E8 03"); + CHECK_BODY("10 RESUME 50", "A7 20 0E 32 00"); + CHECK_BODY("10 ON ERROR GOTO 900", "95 20 A6 20 89 20 0E 84 03"); + // After the jump target, ordinary integers again. + CHECK_BODY("10 GOTO 10:A=10", "89 20 0E 0A 00 3A 41 EF 0F 0A"); +} + +TEST(basic_else_and_apostrophe_carry_a_colon) { + CHECK_BODY("10 IF A THEN 10 ELSE 20", "8B 20 41 20 DA 20 0E 0A 00 20 3A A1 20 0E 14 00"); + CHECK_BODY("10 CLS ' clear", "9F 20 3A 8F E6 20 63 6C 65 61 72"); + CHECK_BODY("10 REM Print 5:GOTO 1", "8F 20 50 72 69 6E 74 20 35 3A 47 4F 54 4F 20 31"); +} + +TEST(basic_data_is_literal) { + // DATA items are not crunched (nor upper-cased) up to the next ':'. + CHECK_BODY("10 DATA 1,abc,PRINT,\"x:y\":A=1", + "84 20 31 2C 61 62 63 2C 50 52 49 4E 54 2C 22 78 3A 79 22 3A 41 EF 12"); +} + +TEST(basic_strings_are_literal) { + CHECK_BODY("10 PRINT \"goto 10\"", "91 20 22 67 6F 74 6F 20 31 30 22"); + CHECK_BODY("10 PRINT \"open", "91 20 22 6F 70 65 6E"); +} + +TEST(basic_lines_sorted_replaced_deleted) { + Bytes t = tokenizeBasic("30 C\n10 A\n20 B\n10 X\n30\n"); + CHECK_EQ(detokenizeBasic(t), std::string("10 X\n20 B\n")); + // Lines without a number and out-of-range numbers are ignored. + CHECK_EQ(detokenizeBasic(tokenizeBasic("PRINT\n70000 A\n65529 B\n")), std::string("65529 B\n")); + CHECK_EQ(hex(tokenizeBasic("")), std::string("FF 00 00")); +} + +TEST(basic_detokenize_numbers) { + auto line = [](const Bytes& b) { + Bytes t{0xFF, 0x00, 0x80, 0x0A, 0x00}; + t.insert(t.end(), b.begin(), b.end()); + t.push_back(0); t.push_back(0); t.push_back(0); + return detokenizeBasic(t); + }; + CHECK_EQ(line({0x1D, 0x41, 0x15, 0x00, 0x00}), std::string("10 1.5!\n")); + CHECK_EQ(line({0x1F, 0x41, 0x10, 0, 0, 0, 0, 0, 0}), std::string("10 1#\n")); + CHECK_EQ(line({0x1F, 0x3F, 0x10, 0, 0, 0, 0, 0, 0}), std::string("10 .01\n")); + CHECK_EQ(line({0x1F, 0x45, 0x32, 0x76, 0x80, 0, 0, 0, 0}), std::string("10 32768\n")); + CHECK_EQ(line({0x1F, 0x55, 0x10, 0, 0, 0, 0, 0, 0}), std::string("10 1D+20\n")); + CHECK_EQ(line({0x1D, 0x37, 0x25, 0x00, 0x00}), std::string("10 2.5E-10\n")); + CHECK_EQ(line({0x1F, 0x41, 0x31, 0x41, 0x59, 0x26, 0x53, 0x58, 0x98}), std::string("10 3.1415926535898\n")); + CHECK_EQ(line({0x1F, 0x00, 0, 0, 0, 0, 0, 0, 0}), std::string("10 0#\n")); + CHECK_EQ(line({0x1C, 0x00, 0x80}), std::string("10 -32768\n")); + CHECK_EQ(line({0x0C, 0x00, 0xC0}), std::string("10 &HC000\n")); + CHECK_EQ(line({0x0B, 0x0F, 0x00}), std::string("10 &O17\n")); +} + +TEST(basic_roundtrip_text) { + const char* prog = + "10 SCREEN 5:COLOR 15,0,0:CLS\n" + "20 DEFINT A-Z:DIM M(10,10)\n" + "30 FOR I=0 TO 255 STEP 2:PSET (I,I),I MOD 16:NEXT I\n" + "40 A$=INKEY$:IF A$=\"\" THEN 40 ELSE PRINT A$;\n" + "50 X1=X1+.5:Y#=1.5:Z!=2.5!:W=1D+20:V=2.5E-10\n" + "60 ON STRIG(0) GOSUB 1000:STRIG(0) ON\n" + "70 DATA 1,2,three,\"four:five\"\n" + "80 POKE &HF3DB,0:A=&O777:B=&B1010\n" + "90 PRINT USING \"###.##\";3.14159 ' pi\n" + "100 IF A<>B AND NOT C OR D XOR E THEN GOTO 10\n" + "110 BLOAD \"GAME.BIN\",R\n" + "1000 RETURN\n"; + Bytes t = tokenizeBasic(prog); + CHECK_EQ(detokenizeBasic(t), std::string(prog)); + CHECK(tokenizeBasic(detokenizeBasic(t)) == t); +} + +TEST(basic_roundtrip_bytes_preserves_types) { + // Every constant form must come back as the same token after a + // detokenize -> tokenize cycle. + const char* forms[] = { + "10 A=1.5!", "10 A=1#", "10 A=1", "10 A=100000", "10 A=.000001", + "10 A=1E-20", "10 A=1D+60", "10 A=123456.7!", "10 A=12345678901234", + "10 A=0!", "10 A=0#", "10 A=99999#", "10 A=-3.5", + }; + for (const char* f : forms) { + Bytes t = tokenizeBasic(f); + Bytes back = tokenizeBasic(detokenizeBasic(t)); + if (back != t) fprintf(stderr, " form %s -> %s", f, detokenizeBasic(t).c_str()); + CHECK(back == t); + } +} + +TEST(basic_detokenize_non_basic) { + CHECK(detokenizeBasic({}).empty()); + CHECK(detokenizeBasic({0xFE, 1, 2}).empty()); + // Truncated program: must not read past the end. + CHECK_EQ(detokenizeBasic({0xFF, 0x10, 0x80, 0x0A, 0x00, 0x91, 0x1F, 0x41}), + std::string("10 PRINT\x1F" "A\n")); +} + +TEST(file_type_detection) { + CHECK_EQ(detectType({}), MsxFileType::Unknown); + CHECK_EQ(detectType({0xFF, 0, 0}), MsxFileType::BasicTokenized); + CHECK_EQ(detectType({0xFE, 0, 0xC0, 0xFF, 0xC0, 0, 0xC0}), MsxFileType::BloadBinary); + std::string a = "10 PRINT \"HI\"\r\n20 GOTO 10\r\n"; + CHECK_EQ(detectType(Bytes(a.begin(), a.end())), MsxFileType::BasicAscii); + std::string txt = "Hello, this is a README.\n"; + CHECK_EQ(detectType(Bytes(txt.begin(), txt.end())), MsxFileType::Text); + CHECK_EQ(detectType({0x00, 0x01, 0x02, 0x80, 0x90}), MsxFileType::Data); + + BloadHeader h = parseBloadHeader({0xFE, 0x00, 0xC0, 0xFF, 0xC0, 0x10, 0xC0}); + CHECK(h.valid); + CHECK_EQ(h.start, 0xC000); CHECK_EQ(h.end, 0xC0FF); CHECK_EQ(h.exec, 0xC010); + CHECK(!parseBloadHeader({0xFE, 0x00, 0xC0, 0x00, 0xB0, 0, 0}).valid); +} + +TEST(carve_blocks) { + Bytes blob(512, 0xAA); // unknown leader + Bytes bl{0xFE, 0x00, 0xC0, 0x03, 0xC0, 0x00, 0xC0, 1, 2, 3, 4}; + blob.insert(blob.end(), bl.begin(), bl.end()); // BLOAD at 512, 4 payload bytes + Bytes bas = tokenizeBasic("10 PRINT 1\n"); + blob.insert(blob.end(), bas.begin(), bas.end()); // right after the block + blob.insert(blob.end(), 20, 0x55); + auto blocks = carveBlocks(blob); + CHECK_EQ(blocks.size(), (size_t)4); + if (blocks.size() == 4) { + CHECK_EQ(blocks[0].type, CarveType::Unknown); CHECK_EQ(blocks[0].length, 512L); + CHECK_EQ(blocks[1].type, CarveType::Bload); CHECK_EQ(blocks[1].offset, 512L); + CHECK_EQ(blocks[1].length, 11L); CHECK_EQ(blocks[1].exec, 0xC000); + CHECK_EQ(blocks[2].type, CarveType::BasicTokenized); + CHECK_EQ(blocks[2].length, (long)bas.size()); + CHECK_EQ(blocks[3].type, CarveType::Unknown); CHECK_EQ(blocks[3].length, 20L); + } +} From 78d39bef7361063ae9dbe41cbbd9e40c59143b6c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 13:17:54 +0000 Subject: [PATCH 3/8] dmk: verify CRCs, flag duplicate IDs, keep LBAs stable on odd tracks - ID and data CRCs (CRC-16/CCITT, MFM 0xA1 preset) are now actually checked; before, idCrcError/dataCrcError were never set although the inspector and README advertise them. - Duplicate sector IDs on a track are flagged (and left unmapped). - Sectors per track come from the BPB, else the most common count of standard sectors. One protected track with an extra sector header used to raise spt for the whole disk and shift every LBA, so the filesystem of such images was unreadable. - Sectors with a data CRC error still map (an FDC returns their bytes). - Writing a logical sector into a DMK recomputes its data CRC, so edited DMKs no longer reload as "protected"; deliberate CRC errors are kept. - dskToDmk() moves into core and emits a proper IBM MFM layout (gaps, syncs, index mark, valid CRCs); tools/dsk2dmk is a thin wrapper. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CtBtjm9PRm6sDhDVJbtciC --- CMakeLists.txt | 1 + core/disk_image.cpp | 188 +++++++++++++++++++++++++++----- core/disk_image.h | 14 +++ tests/test_disk.cpp | 257 ++++++++++++++++++++++++++++++++++++++++++++ tools/dsk2dmk.cpp | 75 +++---------- 5 files changed, 448 insertions(+), 87 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 08746e9..b09fb7d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,6 +19,7 @@ target_link_libraries(msxforge PRIVATE msxcore) # --- dsk2dmk test tool --- add_executable(dsk2dmk tools/dsk2dmk.cpp) +target_link_libraries(dsk2dmk PRIVATE msxcore) # --- tests (core library, no GUI deps): ctest --test-dir build --- option(MSXFF_BUILD_TESTS "Build the core test suite" ON) diff --git a/core/disk_image.cpp b/core/disk_image.cpp index ab30384..26d9e2b 100644 --- a/core/disk_image.cpp +++ b/core/disk_image.cpp @@ -58,7 +58,7 @@ bool DiskImage::loadBytes(std::vector bytes, const std::string& nameHin path_ = nameHint; format_ = DiskFormat::Unknown; raw_ = std::move(bytes); - sectors_.clear(); lba_.clear(); + sectors_.clear(); lba_.clear(); lbaSector_.clear(); cylinders_ = heads_ = spt_ = 0; protectedFlags_ = false; dirty_ = false; if (err) err->clear(); @@ -67,7 +67,7 @@ bool DiskImage::loadBytes(std::vector bytes, const std::string& nameHin if (hasExt(nameHint, ".dmk")) { if (parseDmk(err)) { format_ = DiskFormat::DMK; return true; } // Fall through: some ".dmk" files are actually plain dumps. - sectors_.clear(); lba_.clear(); + sectors_.clear(); lba_.clear(); lbaSector_.clear(); cylinders_ = heads_ = spt_ = 0; protectedFlags_ = false; } if (parsePlain(err)) { if (err) err->clear(); return true; } @@ -152,6 +152,19 @@ bool DiskImage::parsePlain(std::string* err) { // Each track: 0x80-byte IDAM pointer table (64 x uint16 LE, bit15=MFM, // bit14=density), each pointing (relative to track start) at the IDAM (0xFE) // inside the track body. We walk those to recover every sector. +// +// CRCs are CRC-16/CCITT (poly 0x1021, init 0xFFFF). For MFM the three 0xA1 +// sync bytes before each mark are included, which is the same as starting +// from 0xCDB4 at the mark byte itself. + +uint16_t crc16Ccitt(const uint8_t* p, size_t n, uint16_t crc) { + for (size_t i = 0; i < n; ++i) { + crc ^= (uint16_t)(p[i] << 8); + for (int k = 0; k < 8; ++k) crc = (crc & 0x8000) ? (uint16_t)((crc << 1) ^ 0x1021) : (uint16_t)(crc << 1); + } + return crc; +} + bool DiskImage::parseDmk(std::string* err) { if (raw_.size() < 16 + 0x80) { if (err) *err = "dmk too small"; return false; } const uint8_t* h = raw_.data(); @@ -171,23 +184,22 @@ bool DiskImage::parseDmk(std::string* err) { heads_ = sides; cylinders_ = ntracks; - // First pass: collect every physical sector; second pass assigns LBAs. - int maxSecPerTrack = 0; - struct Tmp { SectorInfo si; }; std::vector phys; + std::vector perTrack; // standard (512-byte, in-place) sectors per track for (int t = 0; t < ntracks; ++t) { for (int s = 0; s < sides; ++s) { long trackOff = 16 + ((long)t * sides + s) * trackLen; if (trackOff + trackLen > (long)raw_.size()) break; const uint8_t* tk = raw_.data() + trackOff; - int secOnTrack = 0; + int standard = 0; + size_t firstOnTrack = phys.size(); for (int i = 0; i < 64; ++i) { uint16_t ent = rd16(tk + i * 2); if (ent == 0) continue; int idamPtr = ent & 0x3FFF; // offset within track of the IDAM bool mfm = (ent & 0x8000) != 0; - if (idamPtr + 7 >= trackLen) { protectedFlags_ = true; continue; } + if (idamPtr < 0x80 || idamPtr + 7 > trackLen) { protectedFlags_ = true; continue; } const uint8_t* idam = tk + idamPtr; if (idam[0] != 0xFE) { protectedFlags_ = true; continue; } SectorInfo si; @@ -196,58 +208,174 @@ bool DiskImage::parseDmk(std::string* err) { si.sec = idam[3]; si.sizeCode = idam[4] & 3; si.size = (size_t)128 << si.sizeCode; - if (si.size != 512) { si.nonStandard = true; protectedFlags_ = true; } - if (si.cyl != t || si.head != s) { si.nonStandard = true; protectedFlags_ = true; } + uint16_t idCrc = crc16Ccitt(idam, 5, mfm ? 0xCDB4 : 0xFFFF); + si.idCrcError = idCrc != (uint16_t)((idam[5] << 8) | idam[6]); + if (si.size != 512) si.nonStandard = true; + if (si.cyl != t || si.head != s) si.nonStandard = true; // Locate the data address mark (0xFB normal, 0xF8 deleted) that // follows the IDAM's CRC. Scan a short window forward. long dataOff = -1; int scanStart = idamPtr + 7; int scanEnd = std::min(trackLen, scanStart + (mfm ? 60 : 45)); + int damPos = -1; for (int p = scanStart; p < scanEnd; ++p) { uint8_t v = tk[p]; if (v == 0xFB || v == 0xF8) { - if (v == 0xF8) { si.deleted = true; protectedFlags_ = true; } + if (v == 0xF8) si.deleted = true; dataOff = trackOff + p + 1; + damPos = p; break; } } - if (dataOff < 0) { si.nonStandard = true; protectedFlags_ = true; } - else if (dataOff + (long)si.size <= (long)raw_.size()) si.offset = dataOff; - else { si.nonStandard = true; protectedFlags_ = true; } + si.mfm = mfm; + if (dataOff < 0) si.nonStandard = true; + else if (damPos + 1 + (long)si.size + 2 <= (long)trackLen) { + si.offset = dataOff; + si.markOffset = trackOff + damPos; + uint16_t dc = crc16Ccitt(tk + damPos, 1 + si.size, mfm ? 0xCDB4 : 0xFFFF); + const uint8_t* stored = tk + damPos + 1 + si.size; + si.dataCrcError = dc != (uint16_t)((stored[0] << 8) | stored[1]); + } else if (dataOff + (long)si.size <= (long)raw_.size()) { + si.offset = dataOff; // runs off the track: no CRC to check + si.nonStandard = true; + } else si.nonStandard = true; + // The same sector ID twice on one track is a classic protection. + for (size_t k = firstOnTrack; k < phys.size(); ++k) { + if (phys[k].cyl == si.cyl && phys[k].head == si.head && phys[k].sec == si.sec) { + si.duplicateId = true; si.nonStandard = true; + phys[k].duplicateId = true; phys[k].nonStandard = true; + } + } + if (si.size == 512 && si.offset >= 0 && !si.idCrcError && !si.duplicateId && + si.cyl == t && si.head == s) + standard++; phys.push_back(si); - secOnTrack++; } - maxSecPerTrack = std::max(maxSecPerTrack, secOnTrack); + perTrack.push_back(standard); } } if (phys.empty()) { if (err) *err = "dmk: no sectors found"; return false; } - spt_ = maxSecPerTrack; + for (auto& si : phys) + if (si.nonStandard || si.deleted || si.idCrcError || si.dataCrcError) protectedFlags_ = true; + + // Logical sectors per track: the BPB of a readable boot sector wins; else + // the most common count of standard sectors (a protected track with extra + // or missing sectors must not shift every other track's LBAs). + int spt = 0; + for (auto& si : phys) { + if (si.cyl == 0 && si.head == 0 && si.sec == 1 && si.size == 512 && si.offset >= 0 && + !si.idCrcError) { + const uint8_t* b = raw_.data() + si.offset; + int bpbSpt = rd16(b + 0x18); + if (rd16(b + 0x0B) == 512 && bpbSpt >= 8 && bpbSpt <= 10) spt = bpbSpt; + break; + } + } + if (spt == 0) { + int counts[65] = {0}; + for (int c : perTrack) if (c > 0 && c <= 64) counts[c]++; + for (int c = 1; c <= 64; ++c) if (counts[c] > counts[spt]) spt = c; + if (spt == 0) spt = 9; + } + spt_ = spt; // Build a logical map for the standard-looking sectors so FAT12 can read a // regular filesystem straight out of a protected image. We map by CHS: - // lba = (cyl*heads + head)*spt + (sec-1), only for 512-byte, in-range sectors. - long maxLba = (long)cylinders_ * heads_ * (spt_ > 0 ? spt_ : 9); + // lba = (cyl*heads + head)*spt + (sec-1), only for 512-byte sectors whose + // ID is readable and unique. A data CRC error still maps (a real FDC + // returns the bytes along with the error status). + long maxLba = (long)cylinders_ * heads_ * spt_; lba_.assign((size_t)maxLba, -1); - for (auto& si : phys) { - if (si.size == 512 && si.offset >= 0 && si.sec >= 1 && - !si.dataCrcError && !si.idCrcError) { - long lba = ((long)si.cyl * heads_ + si.head) * (spt_ > 0 ? spt_ : 9) + (si.sec - 1); + lbaSector_.assign((size_t)maxLba, -1); + for (size_t k = 0; k < phys.size(); ++k) { + SectorInfo& si = phys[k]; + if (si.size == 512 && si.offset >= 0 && si.sec >= 1 && si.sec <= spt_ && + si.head < heads_ && !si.idCrcError && !si.duplicateId) { + long lba = ((long)si.cyl * heads_ + si.head) * spt_ + (si.sec - 1); if (lba >= 0 && lba < maxLba && lba_[(size_t)lba] < 0) { lba_[(size_t)lba] = si.offset; + lbaSector_[(size_t)lba] = (int)k; si.lba = lba; } } } // Trim trailing unmapped holes so logicalSectors() reflects real content. - while (!lba_.empty() && lba_.back() < 0) lba_.pop_back(); + while (!lba_.empty() && lba_.back() < 0) { lba_.pop_back(); lbaSector_.pop_back(); } sectors_ = std::move(phys); return true; } +std::vector dskToDmk(const std::vector& dsk, std::string* err) { + std::vector out; + if (dsk.size() < 512) { if (err) *err = "image too small"; return out; } + long tot = (rd16(&dsk[0x0B]) == 512) ? rd16(&dsk[0x13]) : 0; + if (tot <= 0 || (long)dsk.size() < tot * 512) tot = (long)dsk.size() / 512; + int spt, heads; + switch (tot) { + case 1440: spt = 9; heads = 2; break; + case 720: spt = 9; heads = (dsk[0x15] == 0xFD) ? 2 : 1; break; + case 1280: spt = 8; heads = 2; break; + case 640: spt = 8; heads = (dsk[0x15] == 0xFF) ? 2 : 1; break; + case 360: spt = 9; heads = 1; break; + case 320: spt = 8; heads = 1; break; + default: + if (err) *err = "unsupported geometry (" + std::to_string(tot) + " sectors)"; + return out; + } + int cyls = (int)(tot / (spt * heads)); + + const int kTrackLen = 0x1900; // usual DD DMK track size + const int kFixed = 12 + 4 + 4 + 2 + 22 + 12 + 4 + 512 + 2; // per sector, sans gap3 + const int kLead = 80 + 12 + 4 + 50; // gap4a, sync, IAM, gap1 + int gap3 = (kTrackLen - 0x80 - kLead) / spt - kFixed; + if (gap3 > 84) gap3 = 84; + if (gap3 < 10) gap3 = 10; + + uint8_t hdr[16] = {0}; + hdr[1] = (uint8_t)cyls; + hdr[2] = kTrackLen & 0xFF; hdr[3] = (kTrackLen >> 8) & 0xFF; + hdr[4] = (heads == 1) ? 0x10 : 0x00; + out.insert(out.end(), hdr, hdr + 16); + + for (int c = 0; c < cyls; ++c) { + for (int h = 0; h < heads; ++h) { + std::vector tk((size_t)kTrackLen, 0x4E); + memset(tk.data(), 0, 0x80); + size_t p = 0x80; + auto fill = [&](int n, uint8_t v) { for (int i = 0; i < n; ++i) tk[p++] = v; }; + fill(80, 0x4E); fill(12, 0x00); + tk[p++] = 0xC2; tk[p++] = 0xC2; tk[p++] = 0xC2; tk[p++] = 0xFC; // index mark + fill(50, 0x4E); + for (int s = 0; s < spt; ++s) { + fill(12, 0x00); fill(3, 0xA1); + size_t idam = p; + tk[p++] = 0xFE; tk[p++] = (uint8_t)c; tk[p++] = (uint8_t)h; + tk[p++] = (uint8_t)(s + 1); tk[p++] = 0x02; + uint16_t crc = crc16Ccitt(&tk[idam], 5, 0xCDB4); + tk[p++] = (uint8_t)(crc >> 8); tk[p++] = (uint8_t)crc; + fill(22, 0x4E); fill(12, 0x00); fill(3, 0xA1); + size_t dam = p; + tk[p++] = 0xFB; + long src = (((long)c * heads + h) * spt + s) * 512; + if (src + 512 <= (long)dsk.size()) memcpy(&tk[p], &dsk[(size_t)src], 512); + else memset(&tk[p], 0xE5, 512); + p += 512; + crc = crc16Ccitt(&tk[dam], 513, 0xCDB4); + tk[p++] = (uint8_t)(crc >> 8); tk[p++] = (uint8_t)crc; + fill(gap3, 0x4E); + uint16_t ptr = (uint16_t)(idam | 0x8000); // bit15 = MFM + tk[(size_t)s * 2] = ptr & 0xFF; tk[(size_t)s * 2 + 1] = ptr >> 8; + } + out.insert(out.end(), tk.begin(), tk.end()); + } + } + return out; +} + // ---- Logical access ------------------------------------------------------- bool DiskImage::readSector(long lba, uint8_t out[512]) const { @@ -263,6 +391,17 @@ bool DiskImage::writeLogicalSector(long lba, const uint8_t in[512]) { long off = lba_[(size_t)lba]; if (off < 0 || off + 512 > (long)raw_.size()) return false; memcpy(raw_.data() + off, in, 512); + // DMK: keep the data CRC valid, unless the sector carried a deliberate CRC + // error (copy protection), which is preserved as found. + if (format_ == DiskFormat::DMK && (size_t)lba < lbaSector_.size() && lbaSector_[(size_t)lba] >= 0) { + const SectorInfo& si = sectors_[(size_t)lbaSector_[(size_t)lba]]; + if (si.markOffset >= 0 && !si.dataCrcError && + si.markOffset + 1 + 512 + 2 <= (long)raw_.size()) { + uint16_t crc = crc16Ccitt(raw_.data() + si.markOffset, 513, si.mfm ? 0xCDB4 : 0xFFFF); + raw_[(size_t)si.markOffset + 513] = (uint8_t)(crc >> 8); + raw_[(size_t)si.markOffset + 514] = (uint8_t)crc; + } + } dirty_ = true; return true; } @@ -284,6 +423,7 @@ std::string DiskImage::protectionReport() const { if (s.deleted) deleted++; if (s.idCrcError) badId++; if (s.dataCrcError) badData++; + if (s.duplicateId) dup++; } std::ostringstream os; os << "Protection markers detected:"; @@ -291,7 +431,7 @@ std::string DiskImage::protectionReport() const { if (deleted) os << "\n - " << deleted << " deleted-data address mark(s)"; if (badId) os << "\n - " << badId << " ID CRC error(s)"; if (badData) os << "\n - " << badData << " data CRC error(s)"; - (void)dup; + if (dup) os << "\n - " << dup << " sector(s) sharing a duplicate ID"; return os.str(); } diff --git a/core/disk_image.h b/core/disk_image.h index 7737798..d95fb90 100644 --- a/core/disk_image.h +++ b/core/disk_image.h @@ -26,6 +26,16 @@ enum class DiskFormat { Unknown, DSK_Plain, RAW, DMK, HDD }; const char* to_string(DiskFormat f); +// CRC-16/CCITT as used by the WD279x/uPD765 FDCs for ID and data fields. +// Pass 0xCDB4 as `crc` to account for the three MFM 0xA1 sync bytes. +uint16_t crc16Ccitt(const uint8_t* p, size_t n, uint16_t crc = 0xFFFF); + +// Re-encode a flat .dsk (360/720KB, 8 or 9 sectors per track) as a DMK image +// with a standard IBM MFM track layout (gaps, 0xA1 syncs, valid ID and data +// CRCs). Geometry comes from the BPB, else from the image size. Returns an +// empty vector (and sets err) if the geometry cannot be determined. +std::vector dskToDmk(const std::vector& dsk, std::string* err = nullptr); + // Physical description of one sector as it was found on the medium. struct SectorInfo { int cyl = 0, head = 0, sec = 0; // CHS id as recorded in the IDAM @@ -35,8 +45,11 @@ struct SectorInfo { bool deleted = false; // deleted-data address mark (protection) bool idCrcError = false; bool dataCrcError = false; + bool duplicateId = false; // same C/H/R appears twice on the track bool nonStandard = false; // size!=512, dup id, or otherwise irregular long lba = -1; // logical block assigned, or -1 if unmapped + long markOffset = -1; // DMK: data address mark byte (CRC covers it) + bool mfm = false; // DMK: double density (CRC preset 0xCDB4) }; class DiskImage { @@ -98,6 +111,7 @@ class DiskImage { std::vector raw_; std::vector sectors_; std::vector lba_; // lba_[n] = offset of logical sector n, or -1 + std::vector lbaSector_; // lbaSector_[n] = index into sectors_, or -1 int cylinders_ = 0, heads_ = 0, spt_ = 0; bool protectedFlags_ = false; bool dirty_ = false; diff --git a/tests/test_disk.cpp b/tests/test_disk.cpp index f106c1d..a9d18d8 100644 --- a/tests/test_disk.cpp +++ b/tests/test_disk.cpp @@ -1 +1,258 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (C) 2026 Alonso (GS·RUN). MSXFileForge. +// Free software under the GNU GPL v3 (or later) - see LICENSE. Modified +// versions must be distributed under the same license. +// Disk containers: plain .dsk geometry, DMK parsing, CRCs and protection. #include "check.h" +#include "disk_image.h" +#include "fat12.h" +#include "util.h" +#include + +using namespace msx; + +namespace { + +typedef std::vector Bytes; + +Bytes pattern(size_t n, uint8_t seed) { + Bytes v(n); + uint32_t x = 0x12345u * (seed + 1); + for (auto& b : v) { x = x * 1103515245u + 12345u; b = (uint8_t)(x >> 16); } + return v; +} + +// A 720K disk with a few files, as raw .dsk bytes. +Bytes sampleDsk(int sectors = 1440) { + DiskImage dk; Fat12 fs; std::string err; + dk.loadBytes(formatBlankImage(sectors), "s.dsk", &err); + fs.mount(dk, &err); + fs.setTimestamp(0x5A21, 0); + fs.createFile("/", "BOOT.BAS", pattern(700, 1), &err); + fs.createFile("/", "DATA.BIN", pattern(20000, 2), &err); + fs.makeDir("/", "SUB", &err); + fs.createFile("/SUB", "DEEP.TXT", pattern(1500, 3), &err); + return dk.raw(); +} + +// Offset (in the DMK file) of track t/side s and its IDAM pointer table. +long trackOffset(const Bytes& dmk, int t, int s) { + int sides = (dmk[4] & 0x10) ? 1 : 2; + int len = dmk[2] | (dmk[3] << 8); + return 16 + ((long)t * sides + s) * len; +} +long idamOffset(const Bytes& dmk, int t, int s, int idx) { + long to = trackOffset(dmk, t, s); + int ptr = (dmk[(size_t)(to + idx * 2)] | (dmk[(size_t)(to + idx * 2 + 1)] << 8)) & 0x3FFF; + return to + ptr; +} + +bool sameFiles(DiskImage& dk) { + Fat12 fs; std::string err; + if (!fs.mount(dk, &err)) { fprintf(stderr, " mount: %s\n", err.c_str()); return false; } + DirEntry e; + bool ok = fs.lookup("/BOOT.BAS", e) && fs.readFile(e) == pattern(700, 1) && + fs.lookup("/DATA.BIN", e) && fs.readFile(e) == pattern(20000, 2) && + fs.lookup("/SUB/DEEP.TXT", e) && fs.readFile(e) == pattern(1500, 3); + return ok && fs.verify().issues.empty(); +} + +} // namespace + +TEST(crc16_ccitt_known_values) { + const uint8_t check[] = {'1','2','3','4','5','6','7','8','9'}; + CHECK_EQ(crc16Ccitt(check, 9), (uint16_t)0x29B1); // CRC-16/CCITT-FALSE check value + const uint8_t sync[] = {0xA1, 0xA1, 0xA1}; + CHECK_EQ(crc16Ccitt(sync, 3), (uint16_t)0xCDB4); // MFM preset +} + +TEST(crc32_known_value) { + const uint8_t check[] = {'1','2','3','4','5','6','7','8','9'}; + CHECK_EQ(crc32(check, 9), 0xCBF43926u); +} + +TEST(plain_image_geometry) { + DiskImage dk; std::string err; + CHECK(dk.loadBytes(formatBlankImage(1440), "a.dsk", &err)); + CHECK_EQ(dk.format(), DiskFormat::DSK_Plain); + CHECK_EQ(dk.cylinders(), 80); CHECK_EQ(dk.heads(), 2); CHECK_EQ(dk.sectorsPerTrack(), 9); + CHECK_EQ(dk.logicalSectors(), 1440L); + CHECK(!dk.isProtected()); + CHECK(dk.loadBytes(formatBlankImage(720), "b.dsk", &err)); + CHECK_EQ(dk.heads(), 1); + // No boot signature: still readable, classified as a RAW dump. + CHECK(dk.loadBytes(Bytes(737280, 0xE5), "c.dsk", &err)); + CHECK_EQ(dk.format(), DiskFormat::RAW); + CHECK(!dk.loadBytes(Bytes(100, 0), "d.dsk", &err)); + CHECK(!err.empty()); +} + +TEST(plain_sector_read_write) { + DiskImage dk; std::string err; + CHECK(dk.loadBytes(formatBlankImage(1440), "a.dsk", &err)); + uint8_t buf[512]; + memset(buf, 0x5A, sizeof buf); + CHECK(!dk.dirty()); + CHECK(dk.writeLogicalSector(100, buf)); + CHECK(dk.dirty()); + uint8_t back[512]; + CHECK(dk.readSector(100, back)); + CHECK(memcmp(buf, back, 512) == 0); + CHECK(!dk.readSector(1440, back)); + CHECK(!dk.readSector(-1, back)); + CHECK(!dk.writeLogicalSector(1440, buf)); +} + +TEST(dmk_roundtrip_reads_same_filesystem) { + for (int sectors : {1440, 720}) { + Bytes dsk = sampleDsk(sectors); + std::string err; + Bytes dmk = dskToDmk(dsk, &err); + CHECK(!dmk.empty()); + DiskImage dk; + CHECK(dk.loadBytes(dmk, "x.dmk", &err)); + CHECK_EQ(dk.format(), DiskFormat::DMK); + CHECK(!dk.isProtected()); + CHECK_EQ(dk.sectorsPerTrack(), 9); + CHECK_EQ(dk.logicalSectors(), (long)sectors); + for (auto& si : dk.sectors()) { + CHECK(!si.idCrcError); + CHECK(!si.dataCrcError); + } + // Every logical sector equals the source .dsk. + bool same = true; + uint8_t buf[512]; + for (long l = 0; l < sectors; ++l) + if (!dk.readSector(l, buf) || memcmp(buf, &dsk[(size_t)l * 512], 512) != 0) same = false; + CHECK(same); + CHECK(sameFiles(dk)); + } +} + +TEST(dmk_writes_keep_crcs_valid) { + std::string err; + DiskImage dk; + CHECK(dk.loadBytes(dskToDmk(sampleDsk(), &err), "x.dmk", &err)); + { + Fat12 fs; CHECK(fs.mount(dk, &err)); + CHECK(fs.createFile("/", "NEW.BIN", pattern(5000, 9), &err)); + } + // Reload the edited container: no CRC errors may appear. + DiskImage again; + CHECK(again.loadBytes(dk.raw(), "y.dmk", &err)); + CHECK(!again.isProtected()); + Fat12 fs; CHECK(fs.mount(again, &err)); + DirEntry e; CHECK(fs.lookup("/NEW.BIN", e)); + CHECK(fs.readFile(e) == pattern(5000, 9)); + CHECK(sameFiles(again)); +} + +TEST(dmk_detects_id_crc_error) { + std::string err; + Bytes dmk = dskToDmk(sampleDsk(), &err); + long idam = idamOffset(dmk, 10, 1, 4); // track 10 side 1, sector 5 + dmk[(size_t)idam + 5] ^= 0xFF; // break the ID CRC + DiskImage dk; + CHECK(dk.loadBytes(dmk, "x.dmk", &err)); + CHECK(dk.isProtected()); + int bad = 0; + for (auto& si : dk.sectors()) if (si.idCrcError) { bad++; CHECK_EQ(si.lba, -1L); } + CHECK_EQ(bad, 1); + // The unreadable sector is a hole; everything else still maps. + uint8_t buf[512]; + CHECK(!dk.readSector((10 * 2 + 1) * 9 + 4, buf)); + CHECK(dk.readSector((10 * 2 + 1) * 9 + 5, buf)); + CHECK(dk.protectionReport().find("ID CRC") != std::string::npos); +} + +TEST(dmk_detects_data_crc_error_but_keeps_data) { + std::string err; + Bytes dsk = sampleDsk(); + Bytes dmk = dskToDmk(dsk, &err); + long idam = idamOffset(dmk, 2, 0, 0); + // Corrupt the stored data CRC of that sector: DAM is 37 bytes past the ID CRC. + long dam = idam + 7 + 22 + 12 + 3; + CHECK_EQ(dmk[(size_t)dam], (uint8_t)0xFB); + dmk[(size_t)dam + 513] ^= 0x01; + DiskImage dk; + CHECK(dk.loadBytes(dmk, "x.dmk", &err)); + CHECK(dk.isProtected()); + int bad = 0; + for (auto& si : dk.sectors()) if (si.dataCrcError) bad++; + CHECK_EQ(bad, 1); + uint8_t buf[512]; + CHECK(dk.readSector(2 * 2 * 9, buf)); + CHECK(memcmp(buf, &dsk[(size_t)(2 * 2 * 9) * 512], 512) == 0); + // Writing a protected sector must not "repair" its deliberate CRC error. + memset(buf, 0x11, sizeof buf); + CHECK(dk.writeLogicalSector(2 * 2 * 9, buf)); + DiskImage again; CHECK(again.loadBytes(dk.raw(), "y.dmk", &err)); + bad = 0; + for (auto& si : again.sectors()) if (si.dataCrcError) bad++; + CHECK_EQ(bad, 1); +} + +TEST(dmk_detects_deleted_data_mark) { + std::string err; + Bytes dmk = dskToDmk(sampleDsk(), &err); + long idam = idamOffset(dmk, 40, 0, 3); + long dam = idam + 7 + 22 + 12 + 3; + dmk[(size_t)dam] = 0xF8; // deleted data address mark + // Fix up the data CRC so only the mark differs. + uint16_t crc = crc16Ccitt(&dmk[(size_t)dam], 513, 0xCDB4); + dmk[(size_t)dam + 513] = (uint8_t)(crc >> 8); dmk[(size_t)dam + 514] = (uint8_t)crc; + DiskImage dk; + CHECK(dk.loadBytes(dmk, "x.dmk", &err)); + CHECK(dk.isProtected()); + int del = 0; + for (auto& si : dk.sectors()) if (si.deleted) { del++; CHECK(!si.dataCrcError); } + CHECK_EQ(del, 1); + CHECK(dk.protectionReport().find("deleted") != std::string::npos); +} + +TEST(dmk_duplicate_ids_and_extra_sectors_do_not_shift_lbas) { + // Regression: one protected track with more sector headers than the rest + // used to raise sectors-per-track for the whole disk, shifting every LBA + // and making the filesystem unreadable. + std::string err; + Bytes dmk = dskToDmk(sampleDsk(), &err); + long to = trackOffset(dmk, 5, 0); + // Add a 10th IDAM pointer on track 5 that repeats sector 1's header. + dmk[(size_t)to + 18] = dmk[(size_t)to + 0]; + dmk[(size_t)to + 19] = dmk[(size_t)to + 1]; + for (bool wipeBpbSpt : {false, true}) { + Bytes img = dmk; + if (wipeBpbSpt) { + // Make the BPB useless for the geometry: fall back to the most + // common per-track count. + long boot = idamOffset(img, 0, 0, 0) + 7 + 22 + 12 + 3 + 1; + img[(size_t)boot + 0x18] = 0; img[(size_t)boot + 0x19] = 0; + } + DiskImage dk; + CHECK(dk.loadBytes(img, "x.dmk", &err)); + CHECK(dk.isProtected()); + CHECK_EQ(dk.sectorsPerTrack(), 9); + int dups = 0; + for (auto& si : dk.sectors()) if (si.duplicateId) dups++; + CHECK_EQ(dups, 2); + CHECK(dk.protectionReport().find("duplicate") != std::string::npos); + uint8_t buf[512]; + CHECK(!dk.readSector(5 * 2 * 9, buf)); // the ambiguous sector is a hole + CHECK(dk.readSector(5 * 2 * 9 + 1, buf)); + CHECK(dk.readSector(1439, buf)); + } +} + +TEST(dmk_bad_extension_falls_back_to_plain) { + std::string err; + DiskImage dk; + CHECK(dk.loadBytes(sampleDsk(), "mislabeled.dmk", &err)); + CHECK_EQ(dk.format(), DiskFormat::DSK_Plain); + CHECK(sameFiles(dk)); +} + +TEST(dmk_rejects_unknown_geometry) { + std::string err; + CHECK(dskToDmk(Bytes(1000 * 512, 0), &err).empty()); + CHECK(!err.empty()); +} diff --git a/tools/dsk2dmk.cpp b/tools/dsk2dmk.cpp index 60200b5..a582049 100644 --- a/tools/dsk2dmk.cpp +++ b/tools/dsk2dmk.cpp @@ -5,84 +5,33 @@ // dsk2dmk - re-encode a flat .dsk into a DMK track image. // // Used to exercise MSXFileForge's DMK reader end-to-end: a disk read through a -// DMK container must yield the exact same filesystem as the original .dsk. Lays -// out standard double-density (MFM) 512-byte sectors, side-major, sector 1..spt. +// DMK container must yield the exact same filesystem as the original .dsk. The +// encoding itself lives in the core library (msx::dskToDmk): standard IBM MFM +// tracks with gaps, 0xA1 syncs and valid ID/data CRCs. +#include "../core/disk_image.h" #include -#include -#include +#include #include -static uint16_t rd16(const uint8_t* p) { return (uint16_t)(p[0] | (p[1] << 8)); } - int main(int argc, char** argv) { if (argc < 3) { fprintf(stderr, "usage: dsk2dmk \n"); return 1; } FILE* f = fopen(argv[1], "rb"); if (!f) { fprintf(stderr, "cannot open %s\n", argv[1]); return 2; } fseek(f, 0, SEEK_END); long n = ftell(f); fseek(f, 0, SEEK_SET); + if (n <= 0) { fclose(f); fprintf(stderr, "empty input\n"); return 2; } std::vector dsk((size_t)n); if (fread(dsk.data(), 1, (size_t)n, f) != (size_t)n) { fclose(f); return 2; } fclose(f); - // Geometry from the BPB (fall back to 720K DS). - int spc = dsk[0x0D], tot = rd16(&dsk[0x13]); - int spt = 9, heads = 2; - if (tot == 720) { heads = 1; spt = 9; } - else if (tot == 1440) { heads = 2; spt = 9; } - else if (tot == 640) { heads = 1; spt = 8; } - else if (tot == 1280) { heads = 2; spt = 8; } - int cyls = tot / (spt * heads); - (void)spc; - fprintf(stderr, "geometry: %dc x %dh x %ds (tot=%d)\n", cyls, heads, spt, tot); - - const int SECSZ = 512; - const int perSec = 7 + 2 + 1 + SECSZ + 2; // IDAM(7) + gap(2) + DAM(1) + data + crc(2) - const int trackLen = 0x80 + spt * perSec + 128; // + tail slack - - std::vector out; - uint8_t hdr[16] = {0}; - hdr[0] = 0x00; // not write protected - hdr[1] = (uint8_t)cyls; // tracks - hdr[2] = trackLen & 0xFF; hdr[3] = (trackLen >> 8) & 0xFF; - hdr[4] = (heads == 1) ? 0x10 : 0x00; // bit4 = single sided - out.insert(out.end(), hdr, hdr + 16); - - for (int c = 0; c < cyls; ++c) { - for (int h = 0; h < heads; ++h) { - std::vector track(trackLen, 0x00); - int idamTable = 0; // write pointers into first 0x80 bytes - int body = 0x80; - for (int s = 0; s < spt; ++s) { - long lba = ((long)c * heads + h) * spt + s; - long srcOff = lba * SECSZ; - int idamPos = body; - track[body++] = 0xFE; // ID address mark - track[body++] = (uint8_t)c; // cyl - track[body++] = (uint8_t)h; // head - track[body++] = (uint8_t)(s + 1); // sector (1-based) - track[body++] = 0x02; // size code (512) - track[body++] = 0x00; // ID CRC hi (dummy) - track[body++] = 0x00; // ID CRC lo - track[body++] = 0x4E; // gap - track[body++] = 0x4E; - track[body++] = 0xFB; // data address mark - if (srcOff + SECSZ <= (long)dsk.size()) - memcpy(&track[body], &dsk[srcOff], SECSZ); - body += SECSZ; - track[body++] = 0x00; // data CRC hi - track[body++] = 0x00; // data CRC lo - // IDAM pointer: offset of 0xFE within track, bit15=MFM. - uint16_t ptr = (uint16_t)(idamPos | 0x8000); - track[idamTable++] = ptr & 0xFF; - track[idamTable++] = (ptr >> 8) & 0xFF; - } - out.insert(out.end(), track.begin(), track.end()); - } - } + std::string err; + std::vector out = msx::dskToDmk(dsk, &err); + if (out.empty()) { fprintf(stderr, "dsk2dmk: %s\n", err.c_str()); return 4; } FILE* o = fopen(argv[2], "wb"); if (!o) { fprintf(stderr, "cannot write %s\n", argv[2]); return 3; } - fwrite(out.data(), 1, out.size(), o); + size_t w = fwrite(out.data(), 1, out.size(), o); fclose(o); - fprintf(stderr, "wrote %s (%zu bytes)\n", argv[2], out.size()); + if (w != out.size()) { fprintf(stderr, "short write\n"); return 3; } + fprintf(stderr, "wrote %s (%zu bytes, %d tracks)\n", argv[2], out.size(), out[1]); return 0; } From 9ed33003ab9abc13faaea003a497b205c8a160e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 13:19:22 +0000 Subject: [PATCH 4/8] rom, tape: fix ASCII16/ASCII8 tie, plain 48/64K ROMs, more TSX blocks - A game that only writes 0x6000/0x7000 tallies ASCII8 and ASCII16 equally and was reported as ASCII8; ties now go to ASCII16, and writes to 0x77FF count for ASCII16. - 48K/64K ROMs with no bank writes are plain, not "Unknown". - tsxToCas skips the remaining TZX block types (0x15, 0x18, 0x19, 0x23, 0x26-0x28, 0x2B) and reports a truncated block instead of reading past the end. - Tests: ROM header/mapper detection on synthetic images; .cas parsing; CAS<->WAV (1200/2400 baud) and CAS<->TSX round trips; error paths. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CtBtjm9PRm6sDhDVJbtciC --- core/rom.cpp | 40 ++++++++------ core/tape.cpp | 8 +++ tests/test_rom.cpp | 67 +++++++++++++++++++++++ tests/test_tape.cpp | 127 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 227 insertions(+), 15 deletions(-) diff --git a/core/rom.cpp b/core/rom.cpp index 0bd1ea0..45efecc 100644 --- a/core/rom.cpp +++ b/core/rom.cpp @@ -23,30 +23,40 @@ static uint16_t rd16(const std::vector& d, size_t o) { // openMSX-style write-target tally: scan for `LD (nnnn),A` (0x32 lo hi) aimed // at each mapper's bank registers and pick the strongest match. +// Konami : 0x6000 0x8000 0xA000 (0x4000-0x5FFF is fixed) +// Konami SCC : 0x5000 0x7000 0x9000 0xB000 +// ASCII8 : 0x6000 0x6800 0x7000 0x7800 +// ASCII16 : 0x6000 0x7000 (often written as 0x77FF) static RomMapper guessMapper(const std::vector& d) { - if (d.size() <= 0x8000 && - ((d.size() >= 2 && d[0] == 'A' && d[1] == 'B') || - (d.size() > 0x4000 && d[0x4000] == 'A' && d[0x4001] == 'B'))) { - // <=32K with a ROM header and no need to bank -> plain. - if (d.size() <= 0x8000) return RomMapper::None; - } + bool header = (d.size() >= 2 && d[0] == 'A' && d[1] == 'B') || + (d.size() > 0x4001 && d[0x4000] == 'A' && d[0x4001] == 'B'); + // <=32K with a ROM header fits in the slot unbanked -> plain. + if (header && d.size() <= 0x8000) return RomMapper::None; int konami = 0, scc = 0, ascii8 = 0, ascii16 = 0; for (size_t i = 0; i + 2 < d.size(); ++i) { if (d[i] != 0x32) continue; // LD (nnnn),A uint16_t a = d[i+1] | (d[i+2] << 8); // Addresses are shared across mapper families, so tally independently. - if (a == 0x6000) { konami++; ascii8++; ascii16++; } - if (a == 0x8000 || a == 0xA000) konami++; - if (a == 0x5000 || a == 0x9000 || a == 0xB000) scc++; - if (a == 0x7000) { scc++; ascii8++; ascii16++; } - if (a == 0x6800 || a == 0x7800) ascii8++; + switch (a) { + case 0x6000: konami++; ascii8++; ascii16++; break; + case 0x8000: case 0xA000: konami++; break; + case 0x5000: case 0x9000: case 0xB000: scc++; break; + case 0x7000: scc++; ascii8++; ascii16++; break; + case 0x6800: case 0x7800: ascii8++; break; + case 0x77FF: ascii16++; break; + default: break; + } } - int best = scc; RomMapper m = RomMapper::KonamiSCC; - if (konami > best) { best = konami; m = RomMapper::Konami; } + // Ties go to the mapper whose registers are a subset of the others' + // (ASCII16 before ASCII8: a pure 0x6000/0x7000 game is ASCII16). + int best = ascii16; RomMapper m = RomMapper::ASCII16; if (ascii8 > best) { best = ascii8; m = RomMapper::ASCII8; } - if (ascii16 > best) { best = ascii16; m = RomMapper::ASCII16; } + if (konami > best) { best = konami; m = RomMapper::Konami; } + if (scc > best) { best = scc; m = RomMapper::KonamiSCC; } if (best == 0) { - if (d.size() <= 0x8000) return RomMapper::None; + // No bank switching found: a 48/64K ROM can still be plain (mirrored + // over pages 0-3); anything bigger must bank somehow. + if (d.size() <= 0x10000) return RomMapper::None; return RomMapper::Unknown; } return m; diff --git a/core/tape.cpp b/core/tape.cpp index 8e0fb3c..b6e267a 100644 --- a/core/tape.cpp +++ b/core/tape.cpp @@ -138,6 +138,13 @@ std::vector tsxToCas(const std::vector& t, std::string* err) { case 0x12: skip = 4; break; // pure tone case 0x13: if (p>=t.size()) return cas; skip = 1 + t[p]*2; break; // pulse seq case 0x14: if (p+10>t.size()) return cas; skip = 10 + (t[p+7]|(t[p+8]<<8)|(t[p+9]<<16)); break; // pure data + case 0x15: if (p+8>t.size()) return cas; skip = 8 + (t[p+5]|(t[p+6]<<8)|(t[p+7]<<16)); break; // direct recording + case 0x18: case 0x19: if (p+4>t.size()) return cas; skip = 4 + rd32(&t[p]); break; // CSW / generalized + case 0x23: skip = 2; break; // jump + case 0x26: if (p+2>t.size()) return cas; skip = 2 + 2 * rd16(&t[p]); break; // call sequence + case 0x27: skip = 0; break; // return + case 0x28: if (p+2>t.size()) return cas; skip = 2 + rd16(&t[p]); break; // select + case 0x2B: if (p+4>t.size()) return cas; skip = 4 + rd32(&t[p]); break; // signal level case 0x20: skip = 2; break; // pause case 0x21: if (p>=t.size()) return cas; skip = 1 + t[p]; break; // group start case 0x22: skip = 0; break; // group end @@ -154,6 +161,7 @@ std::vector tsxToCas(const std::vector& t, std::string* err) { if (err) *err = "unknown TSX block ID 0x" + std::string(1, "0123456789ABCDEF"[(id>>4)&0xF]) + std::string(1, "0123456789ABCDEF"[id&0xF]); return cas; } + if (p + skip > t.size()) { if (err) *err = "truncated TSX block"; return cas; } p += skip; } return cas; diff --git a/tests/test_rom.cpp b/tests/test_rom.cpp index f106c1d..cb4ada2 100644 --- a/tests/test_rom.cpp +++ b/tests/test_rom.cpp @@ -1 +1,68 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (C) 2026 Alonso (GS·RUN). MSXFileForge. +// Free software under the GNU GPL v3 (or later) - see LICENSE. Modified +// versions must be distributed under the same license. +// Cartridge ROM header parsing and mapper detection on synthetic images. #include "check.h" +#include "rom.h" +#include + +using namespace msx; + +namespace { + +// A ROM of `size` bytes with an "AB" header at 0x0000 whose code writes A to +// each address in `regs` (LD (nnnn),A), `times` times each. +std::vector rom(size_t size, std::initializer_list regs, int times = 3) { + std::vector d(size, 0xFF); + d[0] = 'A'; d[1] = 'B'; d[2] = 0x10; d[3] = 0x40; // INIT = 0x4010 + size_t p = 0x10; + for (int t = 0; t < times; ++t) + for (uint16_t a : regs) { d[p++] = 0x3E; d[p++] = (uint8_t)t; d[p++] = 0x32; d[p++] = a & 0xFF; d[p++] = a >> 8; } + d[p++] = 0xC9; + return d; +} + +} // namespace + +TEST(rom_header_fields) { + auto d = rom(0x4000, {}); + d[4] = 0x00; d[5] = 0x50; d[8] = 0x34; d[9] = 0x12; + RomInfo r = analyzeRom(d); + CHECK(r.hasHeaderAt0); + CHECK(!r.hasHeaderAt4000); + CHECK_EQ(r.init, 0x4010); + CHECK_EQ(r.statement, 0x5000); + CHECK_EQ(r.text, 0x1234); + CHECK_EQ(r.pages16k, 1); + CHECK_EQ(r.mapper, RomMapper::None); +} + +TEST(rom_header_at_4000) { + std::vector d(0x8000, 0xFF); + d[0x4000] = 'A'; d[0x4001] = 'B'; d[0x4002] = 0x00; d[0x4003] = 0x80; + RomInfo r = analyzeRom(d); + CHECK(!r.hasHeaderAt0); + CHECK(r.hasHeaderAt4000); + CHECK_EQ(r.init, 0x8000); + CHECK_EQ(r.mapper, RomMapper::None); +} + +TEST(rom_mapper_detection) { + CHECK_EQ(analyzeRom(rom(0x20000, {0x6000, 0x8000, 0xA000})).mapper, RomMapper::Konami); + CHECK_EQ(analyzeRom(rom(0x20000, {0x5000, 0x7000, 0x9000, 0xB000})).mapper, RomMapper::KonamiSCC); + CHECK_EQ(analyzeRom(rom(0x20000, {0x6000, 0x6800, 0x7000, 0x7800})).mapper, RomMapper::ASCII8); + // Regression: a plain 0x6000/0x7000 game tied ASCII8 and ASCII16 and was + // reported as ASCII8. + CHECK_EQ(analyzeRom(rom(0x20000, {0x6000, 0x7000})).mapper, RomMapper::ASCII16); + CHECK_EQ(analyzeRom(rom(0x20000, {0x6000, 0x77FF})).mapper, RomMapper::ASCII16); +} + +TEST(rom_plain_large_and_unknown) { + // 48K/64K without bank writes: plain ROM, not "Unknown". + CHECK_EQ(analyzeRom(rom(0xC000, {})).mapper, RomMapper::None); + CHECK_EQ(analyzeRom(rom(0x10000, {})).mapper, RomMapper::None); + CHECK_EQ(analyzeRom(rom(0x40000, {})).mapper, RomMapper::Unknown); + CHECK_EQ(analyzeRom({}).mapper, RomMapper::None); + CHECK_EQ(analyzeRom({}).size, 0L); +} diff --git a/tests/test_tape.cpp b/tests/test_tape.cpp index f106c1d..57c8408 100644 --- a/tests/test_tape.cpp +++ b/tests/test_tape.cpp @@ -1 +1,128 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (C) 2026 Alonso (GS·RUN). MSXFileForge. +// Free software under the GNU GPL v3 (or later) - see LICENSE. Modified +// versions must be distributed under the same license. +// Cassette: .cas block parsing and .cas <-> .wav / .tsx conversion. #include "check.h" +#include "cas.h" +#include "msxfile.h" +#include "tape.h" +#include + +using namespace msx; + +namespace { + +typedef std::vector Bytes; +const uint8_t kSync[8] = {0x1F,0xA6,0xDE,0xBA,0xCC,0x13,0x7D,0x74}; + +void block(Bytes& cas, const Bytes& payload) { + while (cas.size() % 8) cas.push_back(0x00); // .cas blocks are 8-byte aligned + cas.insert(cas.end(), kSync, kSync + 8); + cas.insert(cas.end(), payload.begin(), payload.end()); +} +Bytes header(uint8_t type, const char* name) { + Bytes h(10, type); + for (int i = 0; i < 6; ++i) h.push_back((uint8_t)(name[i] ? name[i] : ' ')); + return h; +} + +// A cassette with a tokenized BASIC file and a BLOAD binary. +Bytes sampleCas() { + Bytes cas; + block(cas, header(0xD3, "HELLO ")); + Bytes prog = tokenizeBasic("10 PRINT \"HI\"\n20 GOTO 10\n"); + prog.erase(prog.begin()); // the 0xFF is not stored on tape + prog.insert(prog.end(), 7, 0x00); + block(cas, prog); + block(cas, header(0xD0, "GAME ")); + Bytes bin{0x00, 0xC0, 0x0F, 0xC0, 0x00, 0xC0}; + for (int i = 0; i < 16; ++i) bin.push_back((uint8_t)(i * 17)); + block(cas, bin); + return cas; +} + +} // namespace + +TEST(cas_parse_blocks) { + Bytes cas = sampleCas(); + auto bl = parseCas(cas); + CHECK_EQ(bl.size(), (size_t)4); + if (bl.size() == 4) { + CHECK_EQ(bl[0].kind, CasBlockKind::BasicTokenized); + CHECK_EQ(bl[0].filename, std::string("HELLO")); + CHECK_EQ(bl[1].kind, CasBlockKind::Custom); + CHECK_EQ(bl[2].kind, CasBlockKind::Binary); + CHECK_EQ(bl[2].filename, std::string("GAME")); + CHECK_EQ(bl[3].offset % 8, 0L); + CHECK_EQ(bl[3].dataLength, (long)(cas.size() - bl[3].dataOffset)); + } + CHECK(parseCas({}).empty()); + CHECK_EQ(std::string(casKindName(CasBlockKind::Ascii)), std::string("ASCII file")); +} + +TEST(tape_sniffers) { + Bytes w = casToWav(sampleCas()); + CHECK(isWav(w)); CHECK(!isTsx(w)); + Bytes t = casToTsx(sampleCas()); + CHECK(isTsx(t)); CHECK(!isWav(t)); + CHECK(!isWav({'R','I','F','F'})); +} + +TEST(tape_wav_format) { + Bytes w = casToWav(sampleCas()); + CHECK(w.size() > 44); + // 8-bit mono PCM at 43200 Hz, sizes consistent. + auto rd32 = [&](size_t o) { return (uint32_t)(w[o] | (w[o+1] << 8) | (w[o+2] << 16) | ((uint32_t)w[o+3] << 24)); }; + CHECK_EQ(rd32(4), (uint32_t)(w.size() - 8)); + CHECK_EQ(rd32(24), 43200u); + CHECK_EQ((int)w[34], 8); + CHECK_EQ(rd32(40), (uint32_t)(w.size() - 44)); +} + +TEST(tape_cas_wav_roundtrip) { + for (int baud : {1200, 2400}) { + Bytes cas = sampleCas(); + std::string err; + Bytes back = wavToCas(casToWav(cas, baud), &err); + if (back != cas) fprintf(stderr, " baud %d: %zu vs %zu bytes (%s)\n", baud, back.size(), cas.size(), err.c_str()); + CHECK(back == cas); + } +} + +TEST(tape_cas_tsx_roundtrip) { + Bytes cas = sampleCas(); + std::string err; + Bytes tsx = casToTsx(cas); + CHECK(tsxToCas(tsx, &err) == cas); + CHECK(err.empty()); + // Metadata blocks are skipped: insert a text description (0x30) and an + // archive-info block (0x32) after the TZX header. + Bytes meta{0x30, 3, 'M', 'S', 'X', 0x32, 4, 0, 1, 0x00, 1, 'X'}; + tsx.insert(tsx.begin() + 10, meta.begin(), meta.end()); + err.clear(); + CHECK(tsxToCas(tsx, &err) == cas); + CHECK(err.empty()); +} + +TEST(tape_tsx_errors) { + std::string err; + CHECK(tsxToCas({1, 2, 3}, &err).empty()); + CHECK(!err.empty()); + Bytes t = casToTsx(sampleCas()); + t.push_back(0x99); // unknown block id + err.clear(); + CHECK(!tsxToCas(t, &err).empty()); // data up to the bad block survives + CHECK(err.find("0x99") != std::string::npos); + Bytes cut = casToTsx(sampleCas()); + cut.resize(cut.size() - 5); // truncated last 0x4B block + err.clear(); + tsxToCas(cut, &err); + CHECK(!err.empty()); +} + +TEST(tape_wav_errors) { + std::string err; + CHECK(wavToCas({1, 2, 3}, &err).empty()); + CHECK(!err.empty()); +} From 92ebf81b06a91847d72a512701c1a860996b26e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 13:21:43 +0000 Subject: [PATCH 5/8] cli: paths into subdirectories, delete empty dirs, tree-wide diff - cat/detok/extract/put/del/rename/crc accept DIR/NAME paths; before, only root-directory files could be reached from the CLI. - del removes an empty directory; protect shows duplicate-ID sectors. - diff walks the whole tree and exits 1 when the images differ. - save errors are reported instead of exiting silently. - Usage lists every command. - tests/cli_smoke.cmake drives the binary end to end (format, tok, add, mkdir, ls, cat, extract, rename, del, fsck, diff, dsk2dmk, cas, tsx) as a ctest that runs on every platform. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CtBtjm9PRm6sDhDVJbtciC --- cli/main.cpp | 157 +++++++++++++++++++++----------------- tests/CMakeLists.txt | 5 ++ tests/cli_smoke.cmake | 78 +++++++++++++++++++ tests/data/.gitattributes | 3 + tests/data/ascii.cas | 2 + 5 files changed, 177 insertions(+), 68 deletions(-) create mode 100644 tests/cli_smoke.cmake create mode 100644 tests/data/.gitattributes create mode 100644 tests/data/ascii.cas diff --git a/cli/main.cpp b/cli/main.cpp index 84a0e05..5016955 100644 --- a/cli/main.cpp +++ b/cli/main.cpp @@ -4,14 +4,8 @@ // versions must be distributed under the same license. // MSXFileForge CLI - exercises the core library against real images. // -// msxforge info -// msxforge ls [/PATH] [--deleted] -// msxforge cat (raw bytes to stdout) -// msxforge detok (detokenize BASIC to stdout) -// msxforge extract -// msxforge protect (protected/RAW sector inspector) -// msxforge rominfo -// msxforge cas +// Run without arguments for the full command list. Every argument may +// be a path into subdirectories ("GAMES/SAVE/SLOT1.DAT"), case-insensitive. #include "../core/disk_image.h" #include "../core/fat12.h" #include "../core/msxfile.h" @@ -53,14 +47,11 @@ static int mountDisk(const char* path, DiskImage& dk, Fat12& fs) { return 0; } -static const DirEntry* findEntry(const Fat12& fs, const std::string& name, - std::vector& listing) { - std::string want = name; for (auto& c : want) c = (char)toupper((unsigned char)c); - for (auto& e : listing) { - std::string nm = e.name; for (auto& c : nm) c = (char)toupper((unsigned char)c); - if (nm == want) return &e; - } - return nullptr; +// Resolve a FILE argument ("NAME.EXT", "/DIR/NAME.EXT" or "DIR\\NAME.EXT"). +static bool findEntry(const Fat12& fs, const std::string& name, DirEntry& out) { + std::string p = name; + if (p.empty() || (p[0] != '/' && p[0] != '\\')) p = "/" + p; + return fs.lookup(p, out); } static int cmd_info(const char* img) { @@ -113,10 +104,9 @@ static int cmd_cat(int argc, char** argv, bool detok) { if (argc < 4) { fprintf(stderr, "need \n"); return 1; } DiskImage dk; Fat12 fs; int rc = mountDisk(argv[2], dk, fs); if (rc) return rc; - auto root = fs.list("/", false); - const DirEntry* e = findEntry(fs, argv[3], root); - if (!e) { fprintf(stderr, "file not found: %s\n", argv[3]); return 4; } - auto data = fs.readFile(*e); + DirEntry e; + if (!findEntry(fs, argv[3], e)) { fprintf(stderr, "file not found: %s\n", argv[3]); return 4; } + auto data = fs.readFile(e); if (detok) { std::string txt = detokenizeBasic(data); fwrite(txt.data(), 1, txt.size(), stdout); @@ -131,10 +121,9 @@ static int cmd_extract(int argc, char** argv) { if (argc < 5) { fprintf(stderr, "need \n"); return 1; } DiskImage dk; Fat12 fs; int rc = mountDisk(argv[2], dk, fs); if (rc) return rc; - auto root = fs.list("/", false); - const DirEntry* e = findEntry(fs, argv[3], root); - if (!e) { fprintf(stderr, "file not found: %s\n", argv[3]); return 4; } - auto data = fs.readFile(*e); + DirEntry e; + if (!findEntry(fs, argv[3], e)) { fprintf(stderr, "file not found: %s\n", argv[3]); return 4; } + auto data = fs.readFile(e); FILE* out = fopen(argv[4], "wb"); if (!out) { fprintf(stderr, "cannot write %s\n", argv[4]); return 5; } fwrite(data.data(), 1, data.size(), out); @@ -151,15 +140,16 @@ static int cmd_protect(const char* img) { if (dk.isProtected()) printf("%s\n", dk.protectionReport().c_str()); int shown = 0, anomalies = 0; for (auto& s : dk.sectors()) { - bool odd = s.nonStandard || s.deleted || s.idCrcError || s.dataCrcError; + bool odd = s.nonStandard || s.deleted || s.idCrcError || s.dataCrcError || s.duplicateId; if (odd) anomalies++; if (odd && shown < 40) { - printf(" C%02d H%d S%02d size=%zu%s%s%s%s lba=%ld\n", + printf(" C%02d H%d S%02d size=%zu%s%s%s%s%s lba=%ld\n", s.cyl, s.head, s.sec, s.size, s.nonStandard ? " NONSTD" : "", s.deleted ? " DELETED" : "", s.idCrcError ? " IDCRC" : "", - s.dataCrcError ? " DATACRC" : "", s.lba); + s.dataCrcError ? " DATACRC" : "", + s.duplicateId ? " DUPID" : "", s.lba); shown++; } } @@ -172,14 +162,9 @@ static int cmd_put(int argc, char** argv) { if (argc < 5) { fprintf(stderr, "need [outimage]\n"); return 1; } DiskImage dk; Fat12 fs; int rc = mountDisk(argv[2], dk, fs); if (rc) return rc; - auto root = fs.list("/", false); - DirEntry* e = nullptr; - std::string want = argv[3]; for (auto& c : want) c = (char)toupper((unsigned char)c); - for (auto& d : root) { - std::string nm = d.name; for (auto& c : nm) c = (char)toupper((unsigned char)c); - if (nm == want) { e = &d; break; } - } - if (!e) { fprintf(stderr, "file not found: %s\n", argv[3]); return 4; } + DirEntry ent; + if (!findEntry(fs, argv[3], ent)) { fprintf(stderr, "file not found: %s\n", argv[3]); return 4; } + DirEntry* e = &ent; auto data = slurp(argv[4]); std::string err; if (!fs.writeFile(*e, data, &err)) { fprintf(stderr, "write: %s\n", err.c_str()); return 6; } @@ -194,11 +179,11 @@ static int cmd_del(int argc, char** argv) { if (argc < 4) { fprintf(stderr, "need [outimage]\n"); return 1; } DiskImage dk; Fat12 fs; int rc = mountDisk(argv[2], dk, fs); if (rc) return rc; - auto root = fs.list("/", false); - const DirEntry* e = findEntry(fs, argv[3], root); - if (!e) { fprintf(stderr, "file not found: %s\n", argv[3]); return 4; } + DirEntry e; + if (!findEntry(fs, argv[3], e)) { fprintf(stderr, "file not found: %s\n", argv[3]); return 4; } std::string err; - if (!fs.deleteFile(*e, &err)) { fprintf(stderr, "delete: %s\n", err.c_str()); return 6; } + bool ok = e.isDir() ? fs.removeDir(e, &err) : fs.deleteFile(e, &err); + if (!ok) { fprintf(stderr, "delete: %s\n", err.c_str()); return 6; } const char* out = (argc >= 5) ? argv[4] : argv[2]; if (!dk.save(out, &err)) { fprintf(stderr, "save: %s\n", err.c_str()); return 7; } printf("deleted %s -> %s\n", argv[3], out); @@ -313,15 +298,11 @@ static int cmd_add(int argc, char** argv) { static int cmd_rename(int argc, char** argv) { if (argc < 5) { fprintf(stderr, "need \n"); return 1; } DiskImage dk; Fat12 fs; int rc = mountDisk(argv[2], dk, fs); if (rc) return rc; - auto root = fs.list("/", false); - DirEntry* e = nullptr; - std::string want = argv[3]; for (auto& c : want) c = (char)toupper((unsigned char)c); - for (auto& d : root) { std::string nm=d.name; for(auto&c:nm)c=(char)toupper((unsigned char)c); - if (nm==want){e=&d;break;} } - if (!e) { fprintf(stderr, "not found\n"); return 4; } + DirEntry e; + if (!findEntry(fs, argv[3], e)) { fprintf(stderr, "not found: %s\n", argv[3]); return 4; } std::string err; - if (!fs.renameEntry(*e, argv[4], &err)) { fprintf(stderr, "rename: %s\n", err.c_str()); return 6; } - if (!dk.save(argv[2], &err)) return 7; + if (!fs.renameEntry(e, argv[4], &err)) { fprintf(stderr, "rename: %s\n", err.c_str()); return 6; } + if (!dk.save(argv[2], &err)) { fprintf(stderr, "save: %s\n", err.c_str()); return 7; } printf("renamed %s -> %s\n", argv[3], argv[4]); return 0; } @@ -331,7 +312,7 @@ static int cmd_mkdir(int argc, char** argv) { std::string parent = (argc >= 5) ? argv[4] : "/"; std::string err; if (!fs.makeDir(parent, argv[3], &err)) { fprintf(stderr, "mkdir: %s\n", err.c_str()); return 6; } - if (!dk.save(argv[2], &err)) return 7; + if (!dk.save(argv[2], &err)) { fprintf(stderr, "save: %s\n", err.c_str()); return 7; } printf("created dir %s in %s\n", argv[3], parent.c_str()); return 0; } @@ -339,10 +320,9 @@ static int cmd_crc(int argc, char** argv) { if (argc < 3) { fprintf(stderr, "need [FILE]\n"); return 1; } if (argc >= 4) { DiskImage dk; Fat12 fs; int rc = mountDisk(argv[2], dk, fs); if (rc) return rc; - auto root = fs.list("/", false); - const DirEntry* e = findEntry(fs, argv[3], root); - if (!e) { fprintf(stderr, "not found\n"); return 4; } - auto d = fs.readFile(*e); + DirEntry e; + if (!findEntry(fs, argv[3], e)) { fprintf(stderr, "not found: %s\n", argv[3]); return 4; } + auto d = fs.readFile(e); printf("%08X %s (%zu bytes)\n", crc32(d), argv[3], d.size()); } else { auto d = slurp(argv[2]); @@ -364,26 +344,42 @@ static int cmd_fsck(const char* img) { return c.issues.empty() ? 0 : 1; } +// Every file on the disk as "/DIR/NAME" -> entry (directories walked). +static void allFiles(const Fat12& fs, const std::string& dir, + std::vector>& out, int depth = 0) { + if (depth > 16) return; // corrupt trees can loop + for (auto& e : fs.list(dir, false)) { + if (e.isVolume() || e.name == "." || e.name == "..") continue; + std::string p = (dir == "/" ? "" : dir) + "/" + e.name; + if (e.isDir()) allFiles(fs, p, out, depth + 1); + else out.push_back({p, e}); + } +} + static int cmd_diff(int argc, char** argv) { if (argc < 4) { fprintf(stderr, "need \n"); return 1; } DiskImage a, b; Fat12 fa, fb; int rc; if ((rc = mountDisk(argv[2], a, fa))) return rc; if ((rc = mountDisk(argv[3], b, fb))) return rc; - auto la = fa.list("/", false), lb = fb.list("/", false); - auto findIn = [](std::vector& l, const std::string& n) -> DirEntry* { - for (auto& e : l) if (e.name == n && !e.isDir()) return &e; return nullptr; }; + std::vector> la, lb; + allFiles(fa, "/", la); allFiles(fb, "/", lb); + auto findIn = [](std::vector>& l, const std::string& n) -> DirEntry* { + for (auto& e : l) if (e.first == n) return &e.second; + return nullptr; + }; printf("--- %s vs +++ %s\n", baseName(argv[2]).c_str(), baseName(argv[3]).c_str()); - int same=0, diff=0, onlyA=0, onlyB=0; - for (auto& e : la) { if (e.isDir()||e.isVolume()) continue; - DirEntry* o = findIn(lb, e.name); - if (!o) { printf(" - only in A: %s\n", e.name.c_str()); onlyA++; } - else { uint32_t ca=crc32(fa.readFile(e)), cb=crc32(fb.readFile(*o)); - if (ca!=cb) { printf(" * differs : %s (%08X vs %08X)\n", e.name.c_str(), ca, cb); diff++; } - else same++; } } - for (auto& e : lb) { if (e.isDir()||e.isVolume()) continue; - if (!findIn(la, e.name)) { printf(" + only in B: %s\n", e.name.c_str()); onlyB++; } } + int same = 0, diff = 0, onlyA = 0, onlyB = 0; + for (auto& e : la) { + DirEntry* o = findIn(lb, e.first); + if (!o) { printf(" - only in A: %s\n", e.first.c_str()); onlyA++; continue; } + uint32_t ca = crc32(fa.readFile(e.second)), cb = crc32(fb.readFile(*o)); + if (ca != cb) { printf(" * differs : %s (%08X vs %08X)\n", e.first.c_str(), ca, cb); diff++; } + else same++; + } + for (auto& e : lb) + if (!findIn(la, e.first)) { printf(" + only in B: %s\n", e.first.c_str()); onlyB++; } printf("summary: %d same, %d differ, %d only-A, %d only-B\n", same, diff, onlyA, onlyB); - return 0; + return (diff || onlyA || onlyB) ? 1 : 0; } static int cmd_tape(int argc, char** argv, const char* mode) { @@ -434,9 +430,34 @@ int main(int argc, char** argv) { if (argc < 3) { fprintf(stderr, "MSXFileForge CLI\n" - "usage: msxforge [args]\n" - " info|ls|cat|detok|extract|protect \n" - " rominfo cas \n"); + "usage: msxforge [args] (FILE may be a path: DIR/NAME.EXT)\n" + "\n" + "disk images (.dsk / .dmk):\n" + " info geometry, filesystem, free space\n" + " ls [/PATH] [--deleted] list a directory\n" + " cat raw file bytes to stdout\n" + " detok tokenized BASIC as text\n" + " extract copy a file to the host\n" + " add [/DIR] [NAME] add a new file\n" + " put [outimg] overwrite a file\n" + " del [outimg] delete a file or an empty directory\n" + " rename rename in place\n" + " mkdir [/PARENT] create a directory\n" + " format [360|720] blank MSX-DOS disk\n" + " fsck FAT integrity check\n" + " diff compare all files by CRC\n" + " crc [FILE] CRC32 of an image or a file in it\n" + " protect copy-protection / sector inspector\n" + " raw [out.bin] cluster map, dump RAW unnamed data\n" + " rawput [outimg] write RAW unnamed data back\n" + " carve split RAW data into BLOAD/BASIC blocks\n" + "\n" + "other files:\n" + " tok [out.bas] tokenize an ASCII listing\n" + " rominfo header + mapper guess\n" + " cas cassette block list\n" + " cas2wav|cas2tsx [-2] encode tape (-2 = 2400 baud)\n" + " wav2cas|tsx2cas decode tape\n"); return 1; } std::string cmd = argv[1]; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 95f1a8d..6ff2c48 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -3,3 +3,8 @@ add_executable(msxff_tests target_link_libraries(msxff_tests PRIVATE msxcore) add_test(NAME core COMMAND msxff_tests) set_tests_properties(core PROPERTIES TIMEOUT 120) +add_test(NAME cli_smoke COMMAND ${CMAKE_COMMAND} + -DMSXFORGE=$ -DDSK2DMK=$ + -DWORK=${CMAKE_CURRENT_BINARY_DIR}/cli_smoke -DDATA=${CMAKE_CURRENT_SOURCE_DIR}/data + -P ${CMAKE_CURRENT_SOURCE_DIR}/cli_smoke.cmake) +set_tests_properties(cli_smoke PROPERTIES TIMEOUT 120) diff --git a/tests/cli_smoke.cmake b/tests/cli_smoke.cmake new file mode 100644 index 0000000..cd2f9d0 --- /dev/null +++ b/tests/cli_smoke.cmake @@ -0,0 +1,78 @@ +# SPDX-License-Identifier: GPL-3.0-or-later +# End-to-end check of the msxforge CLI (cross-platform: plain CMake script). +# cmake -DMSXFORGE= -DDSK2DMK= -DWORK= -DDATA= -P cli_smoke.cmake +cmake_minimum_required(VERSION 3.16) +file(REMOVE_RECURSE "${WORK}") +file(MAKE_DIRECTORY "${WORK}") +set(IMG "${WORK}/t.dsk") + +function(run expect_rc) + execute_process(COMMAND ${ARGN} WORKING_DIRECTORY "${WORK}" + RESULT_VARIABLE rc OUTPUT_VARIABLE out ERROR_VARIABLE err) + if(NOT rc EQUAL expect_rc) + message(FATAL_ERROR "`${ARGN}` returned ${rc} (want ${expect_rc})\n${out}\n${err}") + endif() + set(OUT "${out}" PARENT_SCOPE) +endfunction() + +function(expect_in needle) + string(FIND "${OUT}" "${needle}" pos) + if(pos EQUAL -1) + message(FATAL_ERROR "expected '${needle}' in output:\n${OUT}") + endif() +endfunction() + +file(WRITE "${WORK}/hello.bas" "10 PRINT \"HELLO\"\n20 GOTO 10\n") +file(WRITE "${WORK}/notes.txt" "MSXFileForge smoke test\n") + +run(0 "${MSXFORGE}" format "${IMG}" 720) +run(0 "${MSXFORGE}" tok "${WORK}/hello.bas" "${WORK}/HELLO.BAS") +expect_in("10 PRINT \"HELLO\"") +run(0 "${MSXFORGE}" add "${IMG}" "${WORK}/HELLO.BAS") +run(0 "${MSXFORGE}" mkdir "${IMG}" DOCS) +run(0 "${MSXFORGE}" add "${IMG}" "${WORK}/notes.txt" /DOCS README.TXT) +run(6 "${MSXFORGE}" add "${IMG}" "${WORK}/notes.txt" / WAYTOOLONGNAME.TXT) +run(0 "${MSXFORGE}" ls "${IMG}" /DOCS) +expect_in("README.TXT") +run(0 "${MSXFORGE}" cat "${IMG}" docs/readme.txt) +expect_in("smoke test") +run(0 "${MSXFORGE}" detok "${IMG}" HELLO.BAS) +expect_in("20 GOTO 10") +run(0 "${MSXFORGE}" extract "${IMG}" /DOCS/README.TXT "${WORK}/out.txt") +file(READ "${WORK}/out.txt" back) +file(READ "${WORK}/notes.txt" orig) +if(NOT back STREQUAL orig) + message(FATAL_ERROR "extracted file differs") +endif() +run(0 "${MSXFORGE}" fsck "${IMG}") +expect_in("OK") + +configure_file("${IMG}" "${WORK}/copy.dsk" COPYONLY) +run(0 "${MSXFORGE}" diff "${IMG}" "${WORK}/copy.dsk") +run(0 "${MSXFORGE}" rename "${WORK}/copy.dsk" DOCS/README.TXT INFO.TXT) +run(1 "${MSXFORGE}" diff "${IMG}" "${WORK}/copy.dsk") +expect_in("only in B: /DOCS/INFO.TXT") +run(6 "${MSXFORGE}" del "${WORK}/copy.dsk" DOCS) # not empty +run(0 "${MSXFORGE}" del "${WORK}/copy.dsk" DOCS/INFO.TXT) +run(0 "${MSXFORGE}" del "${WORK}/copy.dsk" DOCS) +run(4 "${MSXFORGE}" cat "${WORK}/copy.dsk" DOCS/INFO.TXT) +run(0 "${MSXFORGE}" fsck "${WORK}/copy.dsk") + +run(0 "${DSK2DMK}" "${IMG}" "${WORK}/t.dmk") +run(0 "${MSXFORGE}" info "${WORK}/t.dmk") +expect_in("DMK") +expect_in("Protected : no") +run(0 "${MSXFORGE}" cat "${WORK}/t.dmk" DOCS/README.TXT) +expect_in("smoke test") + +configure_file("${DATA}/ascii.cas" "${WORK}/t.cas" COPYONLY) # ASCII header + one data block +run(0 "${MSXFORGE}" cas "${WORK}/t.cas") +expect_in("ASCII file") +run(0 "${MSXFORGE}" cas2tsx "${WORK}/t.cas" "${WORK}/t.tsx") +run(0 "${MSXFORGE}" tsx2cas "${WORK}/t.tsx" "${WORK}/back.cas") +file(SHA256 "${WORK}/t.cas" h1) +file(SHA256 "${WORK}/back.cas" h2) +if(NOT h1 STREQUAL h2) + message(FATAL_ERROR "cas -> tsx -> cas changed the bytes") +endif() +message(STATUS "cli smoke test passed") diff --git a/tests/data/.gitattributes b/tests/data/.gitattributes new file mode 100644 index 0000000..4fb32fe --- /dev/null +++ b/tests/data/.gitattributes @@ -0,0 +1,3 @@ +*.cas binary +*.dsk binary +*.dmk binary diff --git a/tests/data/ascii.cas b/tests/data/ascii.cas new file mode 100644 index 0000000..c41133d --- /dev/null +++ b/tests/data/ascii.cas @@ -0,0 +1,2 @@ +¦ÞºÌ}têêêêêêêêêêREADME¦ÞºÌ}t10 PRINT "HI" + \ No newline at end of file From cca95e481819699983bf626150858cdd57e4e68b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 13:23:30 +0000 Subject: [PATCH 6/8] ci, docs: run the test suites in CI, document headless build and tests - CI runs ctest after every build (the cross-compiled macOS x86_64 binary cannot run on the arm64 runner, so it is skipped there). - README: -DMSXFF_BUILD_GUI=OFF, the test suites, path arguments, and limitations updated (CRC checks and file creation now exist). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CtBtjm9PRm6sDhDVJbtciC --- .github/workflows/build.yml | 7 ++++++ README.md | 48 ++++++++++++++++++++++++++++--------- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 04817eb..eb9e746 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -57,6 +57,13 @@ jobs: shell: bash run: cmake --build build --config Release -j 3 + # The x86_64 macOS build is cross-compiled on an arm64 runner, so its + # test binary cannot run there; every other build runs the suite. + - name: Test + if: matrix.mac_arch != 'x86_64' + shell: bash + run: ctest --test-dir build -C Release --output-on-failure + - name: Verify arch (macOS) if: runner.os == 'macOS' run: | diff --git a/README.md b/README.md index 7803e0b..f40cacc 100644 --- a/README.md +++ b/README.md @@ -81,8 +81,31 @@ to interested packagers. All targets produce `msxforge` (CLI), `msxforge-gui` (GUI) and `dsk2dmk`. +**Headless (no SDL2/OpenGL)** — build only the core, the CLI and the tests: + +```sh +cmake -B build -DMSXFF_BUILD_GUI=OFF && cmake --build build -j +``` + +## Tests + +`ctest --test-dir build --output-on-failure` runs two suites (CI runs them on +Linux, macOS arm64 and Windows): + +- **core** (`tests/test_*.cpp`, no external framework): FAT12 create / overwrite + / delete / rename / mkdir / rmdir, disk-full rollback, 8.3 validation, fsck on + corrupted trees; the BASIC tokenizer byte-for-byte (constants, line numbers, + `DATA`, `REM`/`'`, `ELSE`) and detokenize → tokenize round trips; DMK CRCs, + deleted marks, duplicate IDs and odd tracks; ROM mapper detection; CAS ↔ WAV + and CAS ↔ TSX round trips. `build/tests/msxff_tests ` runs a subset. +- **cli_smoke** (`tests/cli_smoke.cmake`): drives the `msxforge` binary end to + end on a freshly formatted disk. + ## CLI usage +Every `` may be a path into subdirectories (`GAMES/SAVE/SLOT1.DAT`), +case-insensitive. Run `msxforge` without arguments for this list. + ``` msxforge info disk/format/geometry/FAT summary msxforge ls [/PATH] [--deleted] @@ -91,7 +114,7 @@ msxforge detok detokenize BASIC to stdout msxforge tok [out.bas] tokenize text BASIC (round-trip check) msxforge extract msxforge put [outimage] overwrite a file -msxforge del [outimage] delete a file +msxforge del [outimage] delete a file or an empty directory msxforge protect physical sector / protection inspector msxforge raw [out.bin] cluster map (named/RAW/free) + extract RAW data msxforge carve split the RAW region into BLOAD/BASIC/gap blocks @@ -102,7 +125,7 @@ msxforge rename msxforge mkdir [/PARENT] msxforge crc [FILE] CRC32 of a file or the whole image msxforge fsck filesystem integrity check -msxforge diff compare two disks +msxforge diff compare all files of two disks (exit 1 if they differ) msxforge cas2wav [-2] render tape to WAV (-2 = 2400 baud) msxforge cas2tsx [-2] convert to TSX (TZX + MSX 0x4B KCS) msxforge tsx2cas extract tape data from TSX @@ -147,15 +170,18 @@ detokenize, and delete frees clusters and drops the entry. ## Honest limitations -- DMK **CRC verification** is not yet performed (CRC fields are parsed but not - recomputed); anomaly flags (non-standard size, deleted marks, geometry - mismatch) are exercised by construction, not yet against a commercial - copy-protected dump. -- BASIC **BCD floats** are decoded/encoded numerically — correct in value but not - guaranteed byte-identical to the ROM lister's exact rounding. -- Creating brand-new files / renaming (as opposed to overwrite/delete) and - writing back into a `.dmk` container are the next milestones (disk writes - target flat `.dsk`/raw today). +- DMK ID/data **CRCs are verified** (and recomputed when a sector is written), + and duplicate IDs / deleted marks are flagged, but this has been exercised on + constructed images (`tests/test_disk.cpp`), not yet on a library of + commercial copy-protected dumps. Single-density (FM) DMK tracks are parsed but + not specifically tested. +- BASIC constants are converted exactly through their decimal digits. When a + listing is produced, single-precision constants carry a `!` and integral + double-precision constants that would otherwise read back as integers carry a + `#`, so detokenize → tokenize reproduces the bytes; the real MSX `LIST` may + print some of these without the suffix. +- Directories are created, grown and removed; the root directory keeps its fixed + size (112 entries on standard disks), as on MSX-DOS. ## License From 5403bd69a5949c1e5bd64dc3b885c678890a740f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 13:25:09 +0000 Subject: [PATCH 7/8] gui: "Remove this folder" button for empty subdirectories Uses the new Fat12::removeDir(); shown while browsing a subdirectory, goes back up to the parent on success. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CtBtjm9PRm6sDhDVJbtciC --- gui/main.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gui/main.cpp b/gui/main.cpp index 0ea60e6..e6f3971 100644 --- a/gui/main.cpp +++ b/gui/main.cpp @@ -665,6 +665,20 @@ static void drawFileTree(App& a) { a.nameBuf[sizeof a.nameBuf - 1] = 0; a.openRename = true; } if (!canRename) ImGui::EndDisabled(); + if (a.cwd != "/") { + ImGui::SameLine(); + if (ImGui::Button("Remove this folder")) { + DirEntry dir; std::string err; + if (!a.fs.lookup(a.cwd, dir)) a.status = "Remove failed: folder not found"; + else if (a.fs.removeDir(dir, &err)) { + a.status = "Removed folder " + a.cwd + " (image modified)"; + size_t slash = a.cwd.find_last_of('/'); + a.cwd = (slash == 0 || slash == std::string::npos) ? "/" : a.cwd.substr(0, slash); + refreshListing(a); + } else a.status = "Remove failed: " + err; + } + help("Delete the folder you are in. It must be empty (only . and ..). Save the image to persist."); + } ImGui::Separator(); // Modals. From 941c002900272df895f174a457a84d5b0fb08183 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 13:35:56 +0000 Subject: [PATCH 8/8] fat12, dmk, cli: fix issues found in review - Entries in a subdirectory's 2nd+ cluster got the offset of the first cluster plus their index, i.e. the wrong place on disk whenever the clusters are not adjacent (normal once a directory grows). put, del, rename and rmdir then wrote into other files' data. Directories are now parsed cluster by cluster with each cluster's own offset. - lookup() no longer resolves "." / ".."; removeDir, renameEntry and deleteFile refuse dot entries (del SUB/. freed SUB under its parent). - A failed writeFile releases the clusters it allocated; mount caps the sector count at the image size (media-byte guesses on short dumps addressed sectors past the end). - chainOf() stops at a cluster it already visited, so writing a file with a looping FAT chain repairs it instead of freeing it. - createFile/makeDir count the extra cluster only when a full subdirectory must grow (makeDir refused with free slots available; a failed create still grew the directory). - DMK sectors whose data runs past the track end stay readable but are not writable (a write clobbered the next track's IDAM table). - cli diff walks each directory once (a self-referencing directory made it run for ever) and no longer drops files below depth 16. Each fix has a regression test that fails without it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CtBtjm9PRm6sDhDVJbtciC --- cli/main.cpp | 18 ++++--- core/disk_image.cpp | 6 +++ core/fat12.cpp | 76 ++++++++++++++++++++--------- core/fat12.h | 5 +- tests/test_disk.cpp | 24 ++++++++++ tests/test_fat12.cpp | 111 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 209 insertions(+), 31 deletions(-) diff --git a/cli/main.cpp b/cli/main.cpp index 5016955..390874a 100644 --- a/cli/main.cpp +++ b/cli/main.cpp @@ -13,6 +13,7 @@ #include "../core/cas.h" #include "../core/util.h" #include "../core/tape.h" +#include #include #include #include @@ -344,15 +345,19 @@ static int cmd_fsck(const char* img) { return c.issues.empty() ? 0 : 1; } -// Every file on the disk as "/DIR/NAME" -> entry (directories walked). +// Every file on the disk as "/DIR/NAME" -> entry (directories walked once +// each, so a corrupt tree that links back to an ancestor cannot loop). static void allFiles(const Fat12& fs, const std::string& dir, - std::vector>& out, int depth = 0) { - if (depth > 16) return; // corrupt trees can loop + std::vector>& out, + std::vector& seenDirs) { for (auto& e : fs.list(dir, false)) { if (e.isVolume() || e.name == "." || e.name == "..") continue; std::string p = (dir == "/" ? "" : dir) + "/" + e.name; - if (e.isDir()) allFiles(fs, p, out, depth + 1); - else out.push_back({p, e}); + if (e.isDir()) { + if (std::find(seenDirs.begin(), seenDirs.end(), (int)e.startCluster) != seenDirs.end()) continue; + seenDirs.push_back((int)e.startCluster); + allFiles(fs, p, out, seenDirs); + } else out.push_back({p, e}); } } @@ -362,7 +367,8 @@ static int cmd_diff(int argc, char** argv) { if ((rc = mountDisk(argv[2], a, fa))) return rc; if ((rc = mountDisk(argv[3], b, fb))) return rc; std::vector> la, lb; - allFiles(fa, "/", la); allFiles(fb, "/", lb); + std::vector sa, sb; + allFiles(fa, "/", la, sa); allFiles(fb, "/", lb, sb); auto findIn = [](std::vector>& l, const std::string& n) -> DirEntry* { for (auto& e : l) if (e.first == n) return &e.second; return nullptr; diff --git a/core/disk_image.cpp b/core/disk_image.cpp index 26d9e2b..6337e3b 100644 --- a/core/disk_image.cpp +++ b/core/disk_image.cpp @@ -390,6 +390,12 @@ bool DiskImage::writeLogicalSector(long lba, const uint8_t in[512]) { if (lba < 0 || lba >= (long)lba_.size()) return false; long off = lba_[(size_t)lba]; if (off < 0 || off + 512 > (long)raw_.size()) return false; + // DMK: a sector whose data field runs past its track end stays readable, + // but writing it would clobber the next track's header, so refuse. + if (format_ == DiskFormat::DMK) { + if ((size_t)lba >= lbaSector_.size() || lbaSector_[(size_t)lba] < 0) return false; + if (sectors_[(size_t)lbaSector_[(size_t)lba]].markOffset < 0) return false; + } memcpy(raw_.data() + off, in, 512); // DMK: keep the data CRC valid, unless the sector carried a deliberate CRC // error (copy protection), which is preserved as found. diff --git a/core/fat12.cpp b/core/fat12.cpp index fac265b..b8e234c 100644 --- a/core/fat12.cpp +++ b/core/fat12.cpp @@ -119,6 +119,12 @@ bool Fat12::mount(DiskImage& disk, std::string* err) { if (!mountFromMediaByte(err)) return false; } + // Never address sectors the container does not have (a truncated image, or + // a media-byte guess larger than the dump). DMK images may legitimately + // end in unreadable holes, so their logical size is not a cap. + if (disk.format() != DiskFormat::DMK && totalSectors_ > disk.logicalSectors()) + totalSectors_ = (int)disk.logicalSectors(); + fatStartSec_ = rsvd_; rootStartSec_ = rsvd_ + (long)nfat_ * spf_; rootSectors_ = (ndir_ * 32 + bps_ - 1) / bps_; @@ -158,11 +164,12 @@ int Fat12::fatEntry(int cluster) const { } std::vector Fat12::parseEntries(const std::vector& buf, - long baseOffset, bool includeDeleted) { + long baseOffset, bool includeDeleted, bool* ended) { std::vector out; + if (ended) *ended = false; for (size_t i = 0; i + 32 <= buf.size(); i += 32) { const uint8_t* e = buf.data() + i; - if (e[0] == 0x00) break; // end of directory + if (e[0] == 0x00) { if (ended) *ended = true; break; } // end of directory bool del = (e[0] == 0xE5); if (del && !includeDeleted) continue; uint8_t attr = e[0x0B]; @@ -199,20 +206,23 @@ std::vector Fat12::readRootDir(bool includeDeleted) const { } std::vector Fat12::readDirAtCluster(int cluster, bool includeDeleted) const { - std::vector buf; - long firstBase = -1; - int guard = 0; - while (cluster >= 2 && cluster < 0xFF8 && guard++ < totalClusters_ + 4) { - long firstSec = dataStartSec_ + (long)(cluster - 2) * spc_; + // Parse cluster by cluster: a grown directory's clusters are usually not + // adjacent, so each entry's offset must come from its own cluster. + std::vector out; + for (int c : chainOf(cluster)) { + std::vector buf; + long firstSec = dataStartSec_ + (long)(c - 2) * spc_; for (int s = 0; s < spc_; ++s) { uint8_t sec[512]; if (!sectorBytes(firstSec + s, sec)) break; - if (firstBase < 0) firstBase = (firstSec + s) * 512; buf.insert(buf.end(), sec, sec + 512); } - cluster = fatEntry(cluster); + bool ended = false; + auto part = parseEntries(buf, firstSec * 512, includeDeleted, &ended); + out.insert(out.end(), part.begin(), part.end()); + if (ended || buf.size() < (size_t)spc_ * 512) break; } - return parseEntries(buf, firstBase, includeDeleted); + return out; } std::vector Fat12::readDir(int dirCluster, bool includeDeleted) const { @@ -224,7 +234,7 @@ bool Fat12::lookup(const std::string& path, DirEntry& out) const { size_t cut = path.find_last_of("/\\"); std::string dir = (cut == std::string::npos) ? "/" : path.substr(0, cut); std::string leaf = (cut == std::string::npos) ? path : path.substr(cut + 1); - if (leaf.empty()) return false; + if (leaf.empty() || leaf == "." || leaf == "..") return false; int dc = resolveDirCluster(dir); if (dc < 0) return false; for (char& c : leaf) c = (char)toupper((unsigned char)c); @@ -344,9 +354,13 @@ int Fat12::allocCluster() { } std::vector Fat12::chainOf(int startCluster) const { + // Stops at the end marker, at an out-of-range value, or when a corrupt FAT + // loops back to a cluster already in the chain. std::vector chain; - int c = startCluster, guard = 0; - while (c >= 2 && c < 0xFF8 && guard++ < totalClusters_ + 8) { + std::vector seen((size_t)totalClusters_ + 2, false); + int c = startCluster; + while (c >= 2 && c < totalClusters_ + 2 && !seen[(size_t)c]) { + seen[(size_t)c] = true; chain.push_back(c); c = fatEntry(c); } @@ -410,9 +424,13 @@ bool Fat12::writeFile(DirEntry& e, const std::vector& data, std::string return false; } int needClusters = (int)needL; + const size_t oldLen = chain.size(); + auto releaseNew = [&]() { // undo this call's allocations + for (size_t i = oldLen; i < chain.size(); ++i) setFatEntry(chain[i], 0); + }; while ((int)chain.size() < needClusters) { int nc = allocCluster(); - if (nc < 0) { if (err) *err = "disk full (no free clusters)"; return false; } + if (nc < 0) { releaseNew(); if (err) *err = "disk full (no free clusters)"; return false; } setFatEntry(nc, 0xFFF); // reserve immediately so the next alloc skips it chain.push_back(nc); } @@ -422,7 +440,8 @@ bool Fat12::writeFile(DirEntry& e, const std::vector& data, std::string long off = (long)i * clusterBytes; long len = (long)data.size() - off; if (len > clusterBytes) len = clusterBytes; if (!writeClusterData(chain[i], data.data() + off, len)) { - if (err) *err = "write error"; + releaseNew(); + if (err) *err = "write error (sector not writable)"; return false; } } @@ -460,7 +479,10 @@ bool Fat12::writeFile(DirEntry& e, const std::vector& data, std::string bool Fat12::deleteFile(const DirEntry& e, std::string* err) { if (!mounted_) { if (err) *err = "not mounted"; return false; } if (e.isDir()) { if (err) *err = "refusing to delete a directory (use removeDir)"; return false; } - if (e.deleted || e.entryOffset < 0) { if (err) *err = "entry is not a live file"; return false; } + if (e.deleted || e.entryOffset < 0 || (!e.raw83.empty() && e.raw83[0] == '.')) { + if (err) *err = "entry is not a live file"; + return false; + } for (int c : chainOf(e.startCluster)) setFatEntry(c, 0); writeByteAt(e.entryOffset, 0xE5); // mark deleted return true; @@ -468,8 +490,9 @@ bool Fat12::deleteFile(const DirEntry& e, std::string* err) { bool Fat12::removeDir(const DirEntry& e, std::string* err) { if (!mounted_) { if (err) *err = "not mounted"; return false; } - if (!e.isDir() || e.deleted || e.entryOffset < 0 || e.startCluster < 2) { - if (err) *err = "not a subdirectory"; + if (!e.isDir() || e.deleted || e.entryOffset < 0 || e.startCluster < 2 || + (!e.raw83.empty() && e.raw83[0] == '.')) { + if (err) *err = "not a removable subdirectory"; return false; } for (auto& c : readDirAtCluster(e.startCluster, false)) { @@ -630,7 +653,7 @@ int Fat12::resolveDirCluster(const std::string& path) const { return cluster; } -long Fat12::freeDirSlot(int dirCluster) { +long Fat12::freeDirSlot(int dirCluster, bool allowGrow) { if (dirCluster == 0) { for (int i = 0; i < ndir_; ++i) { long off = rootStartSec_ * 512 + (long)i * 32; @@ -650,7 +673,7 @@ long Fat12::freeDirSlot(int dirCluster) { } } // Subdirectories grow: append a zeroed cluster to the directory's chain. - if (chain.empty()) return -1; + if (chain.empty() || !allowGrow) return -1; int nc = allocCluster(); if (nc < 0) return -1; std::vector zero((size_t)spc_ * 512, 0); @@ -685,7 +708,10 @@ bool Fat12::createFile(const std::string& dirPath, const std::string& name, long clusterBytes = (long)spc_ * 512; long need = (long)((data.size() + clusterBytes - 1) / clusterBytes); - if (need > countFreeClusters()) { if (err) *err = "disk full (no free clusters)"; return false; } + // Growing a full subdirectory costs one more cluster: count it up front so + // a failed create leaves the directory as it was. + bool grow = dc != 0 && freeDirSlot(dc, false) < 0; + if (need + (grow ? 1 : 0) > countFreeClusters()) { if (err) *err = "disk full (no free clusters)"; return false; } long slot = freeDirSlot(dc); if (slot < 0) { if (err) *err = "directory is full"; return false; } uint8_t prevFirst = 0; readByteAt(slot, prevFirst); @@ -702,7 +728,10 @@ bool Fat12::createFile(const std::string& dirPath, const std::string& name, } bool Fat12::renameEntry(DirEntry& e, const std::string& newName, std::string* err) { - if (!mounted_ || e.entryOffset < 0 || e.deleted) { if (err) *err = "cannot rename"; return false; } + if (!mounted_ || e.entryOffset < 0 || e.deleted || (!e.raw83.empty() && e.raw83[0] == '.')) { + if (err) *err = "cannot rename"; + return false; + } if (!validName83(newName, err)) return false; uint8_t nm[11]; pack83(newName, nm); // The entry's own directory: find it by scanning for the record offset. @@ -737,7 +766,8 @@ bool Fat12::makeDir(const std::string& parentPath, const std::string& name, std: if (parent < 0) { if (err) *err = "parent directory not found"; return false; } uint8_t nm[11]; pack83(name, nm); if (nameTaken(parent, nm)) { if (err) *err = "name already exists"; return false; } - if (countFreeClusters() < (parent != 0 ? 2 : 1)) { if (err) *err = "disk full"; return false; } + bool grow = parent != 0 && freeDirSlot(parent, false) < 0; + if (countFreeClusters() < (grow ? 2 : 1)) { if (err) *err = "disk full"; return false; } long slot = freeDirSlot(parent); if (slot < 0) { if (err) *err = "parent directory is full"; return false; } int nc = allocCluster(); diff --git a/core/fat12.h b/core/fat12.h index 2c82944..7feb2fb 100644 --- a/core/fat12.h +++ b/core/fat12.h @@ -135,7 +135,7 @@ class Fat12 { private: int resolveDirCluster(const std::string& path) const; // 0=root, -1=not found - long freeDirSlot(int dirCluster); // image offset or -1 + long freeDirSlot(int dirCluster, bool allowGrow = true); // logical offset or -1 void writeDirEntry(long off, const uint8_t name11[11], uint8_t attr, uint16_t cluster, uint32_t size); void stamp(uint16_t& date, uint16_t& time) const; @@ -156,7 +156,8 @@ class Fat12 { int fatEntry(int cluster) const; bool sectorBytes(long lba, uint8_t out[512]) const; static std::vector parseEntries(const std::vector& buf, - long baseOffset, bool includeDeleted); + long baseOffset, bool includeDeleted, + bool* ended = nullptr); DiskImage* disk_ = nullptr; bool mounted_ = false; diff --git a/tests/test_disk.cpp b/tests/test_disk.cpp index a9d18d8..bd3e13d 100644 --- a/tests/test_disk.cpp +++ b/tests/test_disk.cpp @@ -256,3 +256,27 @@ TEST(dmk_rejects_unknown_geometry) { CHECK(dskToDmk(Bytes(1000 * 512, 0), &err).empty()); CHECK(!err.empty()); } + +TEST(dmk_sector_running_off_track_is_read_only) { + std::string err; + Bytes dmk = dskToDmk(sampleDsk(), &err); + long to = trackOffset(dmk, 0, 0); + int len = dmk[2] | (dmk[3] << 8); + // Move sector 9's header near the end of the track so its data runs past it. + long idam = idamOffset(dmk, 0, 0, 8); + long dst = to + len - 20; + memmove(&dmk[(size_t)dst], &dmk[(size_t)idam], 7); + dmk[(size_t)dst + 7] = 0xFB; + uint16_t ptr = (uint16_t)((dst - to) | 0x8000); + dmk[(size_t)to + 16] = ptr & 0xFF; dmk[(size_t)to + 17] = ptr >> 8; + DiskImage dk; + CHECK(dk.loadBytes(dmk, "x.dmk", &err)); + CHECK(dk.isProtected()); + uint8_t buf[512]; + memset(buf, 0x33, sizeof buf); + CHECK(!dk.writeLogicalSector(8, buf)); // would overwrite the next track + CHECK(dk.writeLogicalSector(7, buf)); + DiskImage again; + CHECK(again.loadBytes(dk.raw(), "y.dmk", &err)); + CHECK(again.readSector(9, buf)); // C0 H1 S1 still intact +} diff --git a/tests/test_fat12.cpp b/tests/test_fat12.cpp index d6fed96..415159e 100644 --- a/tests/test_fat12.cpp +++ b/tests/test_fat12.cpp @@ -375,3 +375,114 @@ TEST(fat12_dir_entry_strings) { CHECK_EQ(e.timeString(), std::string("13:45:30")); CHECK_EQ(e.attrString(), std::string("R----A")); } + +// ---- Regressions found in review ------------------------------------------ + +TEST(fat12_entry_offsets_in_split_subdirectory) { + // A directory whose second cluster is not adjacent to the first: entries + // there used to get the offset of the wrong place on disk, so put/delete/ + // rename wrote into another file's data. + Disk d; + std::string err; + CHECK(d.fs.makeDir("/", "SUB", &err)); + auto other = pattern(1024, 77); + CHECK(d.fs.createFile("/", "OTHER.DAT", other, &err)); // the cluster right after SUB + for (int i = 0; i < 31; ++i) { // spills into a new cluster + char nm[16]; snprintf(nm, sizeof nm, "F%02d.TXT", i); + CHECK(d.fs.createFile("/SUB", nm, pattern(10, (uint8_t)i), &err)); + } + DirEntry f; + CHECK(d.find("/SUB/F30.TXT", f)); + CHECK(d.fs.writeFile(f, pattern(3000, 5), &err)); + DirEntry o; CHECK(d.find("/OTHER.DAT", o)); + CHECK(d.fs.readFile(o) == other); + CHECK(d.find("/SUB/F30.TXT", f)); + CHECK(d.fs.readFile(f) == pattern(3000, 5)); + CHECK(clean(d.fs)); + // Delete everything in SUB, then SUB itself. + for (int i = 0; i < 31; ++i) { + char nm[24]; snprintf(nm, sizeof nm, "/SUB/F%02d.TXT", i); + DirEntry e; CHECK(d.find(nm, e)); CHECK(d.fs.deleteFile(e, &err)); + } + CHECK(d.find("/OTHER.DAT", o)); CHECK(d.fs.readFile(o) == other); + DirEntry sub; CHECK(d.find("/SUB", sub)); + CHECK(d.fs.removeDir(sub, &err)); + CHECK_EQ(d.fs.freeBytes(), (713L - 1) * 1024); + CHECK(clean(d.fs)); +} + +TEST(fat12_dot_entries_cannot_be_removed_or_renamed) { + Disk d; + std::string err; + CHECK(d.fs.makeDir("/", "SUB", &err)); + DirEntry e; + CHECK(!d.find("/SUB/.", e)); + CHECK(!d.find("/SUB/..", e)); + for (auto& x : d.fs.list("/SUB")) { + DirEntry y = x; + CHECK(!d.fs.removeDir(x, &err)); + CHECK(!d.fs.renameEntry(y, "LOOP", &err)); + CHECK(!d.fs.deleteFile(x, &err)); + } + CHECK(clean(d.fs)); +} + +TEST(fat12_media_fallback_clamped_to_image_and_failed_write_frees) { + // A 360K dump whose (BPB-less) FAT claims the 720K media byte: mount must + // not address sectors past the end, and a write that cannot fit leaves + // the free space unchanged. + std::string err; + auto img = formatBlankImage(1440); + img.resize(720 * 512); + memset(img.data() + 3, 0, 0x1E - 3); + DiskImage dk; Fat12 fs; + CHECK(dk.loadBytes(img, "half.dsk", &err)); + CHECK(fs.mount(dk, &err)); + CHECK_EQ(fs.totalClusters(), (720 - 1 - 6 - 7) / 2); + long before = fs.freeBytes(); + CHECK(!fs.createFile("/", "BIG.BIN", pattern(500 * 1024, 1), &err)); + CHECK_EQ(fs.freeBytes(), before); + CHECK(fs.verify().issues.empty()); + CHECK_EQ(fs.verify().unreferenced, 0L); +} + +TEST(fat12_write_repairs_looping_chain_instead_of_freeing_it) { + Disk d; + std::string err; + CHECK(d.fs.createFile("/", "A.BIN", pattern(3000, 1), &err)); // clusters 2,3,4 + DirEntry a; CHECK(d.find("/A.BIN", a)); + // Corrupt: last cluster points back to the first. + long fat = 512; + auto& raw = d.dk.rawMut(); + for (int f = 0; f < 2; ++f) { // FAT[4] = 2 (even entry: low 12 bits) + long o = fat + f * 3 * 512 + 4 + 4 / 2; + raw[(size_t)o] = 0x02; raw[(size_t)o + 1] = (uint8_t)((raw[(size_t)o + 1] & 0xF0)); + } + CHECK(!d.fs.verify().issues.empty()); + CHECK(d.fs.writeFile(a, pattern(100, 2), &err)); + CHECK(d.find("/A.BIN", a)); + CHECK(d.fs.readFile(a) == pattern(100, 2)); + CHECK(clean(d.fs)); + CHECK_EQ(d.fs.freeBytes(), (713L - 1) * 1024); +} + +TEST(fat12_full_directory_growth_is_accounted) { + // Fill the disk so exactly one cluster is free. + Disk d(720); + std::string err; + CHECK(d.fs.makeDir("/", "SUB", &err)); + for (int i = 0; i < 30; ++i) { // "." + ".." + 30 = one full cluster + char nm[16]; snprintf(nm, sizeof nm, "F%02d", i); + CHECK(d.fs.createFile("/SUB", nm, {}, &err)); + } + CHECK(d.fs.createFile("/", "FILL.BIN", pattern((size_t)d.fs.freeBytes() - 1024, 1), &err)); + CHECK_EQ(d.fs.freeBytes(), 1024L); + // Creating a 1-cluster file in the full SUB needs 2 clusters: refuse cleanly. + CHECK(!d.fs.createFile("/SUB", "X", pattern(100, 2), &err)); + CHECK_EQ(d.fs.freeBytes(), 1024L); + // A subdirectory with a free slot only needs 1 cluster for a new folder. + DirEntry f; CHECK(d.find("/SUB/F00", f)); CHECK(d.fs.deleteFile(f, &err)); + CHECK(d.fs.makeDir("/SUB", "INNER", &err)); + CHECK_EQ(d.fs.freeBytes(), 0L); + CHECK(clean(d.fs)); +}