smaky6fs: fix size bug, 0xFF deleted entries, decode directory metadata#855
smaky6fs: fix size bug, 0xFF deleted entries, decode directory metadata#855Sch-LikA wants to merge 3 commits intodavidgiven:masterfrom
Conversation
Three bugs fixed by cross-referencing the original Smaky 6 hardware
documentation and independent reverse-engineering of disk images
(including Winchester hard-disk images not previously analysed):
1. Wrong file size when last sector is completely full
When last_bytes == 0 the last sector holds a full 256 bytes;
the old formula (end-start-1)*256 + last_bytes gave a result
256 bytes short in that case.
2. Deleted entries with first byte 0xFF were not skipped
The original check 'if (dbuf[0])' only skipped empty slots
(first byte 0x00). On real Smaky 6 disks and all known Winchester
images, deleted/system entries use 0xFF as a marker. This caused
those entries to be parsed as live files with garbage field values.
3. Directory bytes [0x0e-0x17] were treated as unknown
These fields are fully decodable:
0x0e-0x0f flags (uint16 LE, purpose TBD)
0x10-0x11 last_bytes (bytes used in last sector)
0x12-0x13 load_addr (high byte first; reliable for type SM)
0x14-0x15 entry_addr (same encoding)
0x16 month BCD (0x00/0xFF = no date)
0x17 year BCD (0x82 = 1982; >= 78 -> 19xx, else 20xx)
The raw smaky6.metadata_bytes attribute is replaced by the decoded
smaky6.flags, smaky6.load_addr, smaky6.entry_addr, smaky6.date.
DR (Directory) entries in SAMOS contain a sub-directory in their first 3 sectors, using the same 24-byte entry format as the root directory. Sector addresses within these sub-directories are relative to the DR entry's own start sector. Changes: - SmakyDirent: set file_type = TYPE_DIRECTORY when the filename ends with '.DR', so list/getDirent/getFile can distinguish containers from plain files. - Directory: factor parsing into parseFrom(bytes, sectorBase) and add a second constructor Directory(fs, drStartSector) for sub-directories; relative sector numbers are converted to absolute by adding sectorBase. - list(): handle path.size() == 1 by finding the named DR entry and listing its sub-directory. - getDirent(): handle path.size() == 2 for sub-directory entries. - getFile(): handle path.size() == 2, resolving absolute sector addresses via the sub-directory; guard against calling getFile on a directory. Tested against SM6WIN0.DSK (Winchester): BASIC.DR contains BINBASIC.SM, BINBAS32.SM, BCDBASIC.SM, BCDBAS32.SM at the correct absolute sectors.
| /* Sub-directory inside a DR container: | ||
| * The first 3 sectors of the DR entry hold the sub-directory. | ||
| * Sector numbers stored there are relative to drStartSector. */ | ||
| Directory(Smaky6Filesystem* fs, unsigned drStartSector) |
There was a problem hiding this comment.
You could annotate this as =0 and not need the other constructor.
| return result; | ||
| } | ||
|
|
||
| if (path.size() == 1) |
There was a problem hiding this comment.
Does the filesystem not support nested subdirectories?
| { | ||
| if (path.size() != 1) | ||
| throw BadPathException(path); | ||
| if (path.size() == 1) |
There was a problem hiding this comment.
The directory lookup code here is repeated multiple times. It'd probably be better to have a single method which looks up a path and call it from all these places.
There was a problem hiding this comment.
(Actually, getDirent should already do this?)
…olveDirent helpers - Merge the two Directory constructors into one with drStartSector=0, eliminating the redundant overload (per review comment). - Add private directoryAt(path) and resolveDirent(path) helpers so the repeated two-level path-traversal logic lives in exactly one place. - list(), getDirent() and getFile() now each delegate to those helpers, removing all duplicated root/sub-directory lookup code (per review comment; getDirent now effectively drives the shared resolution).
|
Thanks for the review! I've pushed a new commit (a4a56dd) addressing all three points: Constructor redundancy — merged the two Repeated path-traversal code — added two private helpers:
Nested sub-directories — the Smaky 6 filesystem is structurally flat-within-one-level: a |
2 participants
Three bugs in
lib/vfs/smaky6fs.ccfound by cross-referencing the original Smaky 6 hardware documentation and independent reverse-engineering of disk images (including Winchester hard-disk images not previously analysed).Bug 1 — Wrong file size when last sector is completely full
When
last_bytes == 0the last sector holds a full 256 bytes. The old formula:gives a result 256 bytes short in that case. Fixed to:
Bug 2 — Deleted entries with first byte
0xFFwere not skippedThe original check
if (dbuf[0])only skips empty slots (first byte =0x00). On real Smaky 6 disks and all known Winchester hard-disk images, deleted/system entries use0xFFas a sentinel. This caused those entries to be parsed as live files with garbage field values.Fixed to:
if (dbuf[0] && dbuf[0] != 0xff)Bug 3 — Directory bytes
[0x0e–0x17]were treated as unknownThese fields are fully decodable (documented in the original Smaky 6 hardware manual and confirmed from disk image analysis):
0x0e–0x0fflags0x10–0x11last_bytes0x12–0x13load_addrSM0x14–0x15entry_addr0x16month0x07= July);0x00or0xFF= no date0x17year0x82= 1982); year ≥ 78 → 19xx, else 20xxThe raw
smaky6.metadata_bytesattribute is replaced by the properly decodedsmaky6.flags,smaky6.load_addr,smaky6.entry_addr, andsmaky6.dateattributes.