fix: DOS CD and floppies files and folders access - #2340
Conversation
- Introduced `DosContentEntry` to represent entries in DOS drives. - Refactored `DosDriveBase` to utilize `DosDriveManager` for volume and path management. - Updated `DosFileEntryInfo` to reflect changes in path resolution. - Created `EmptyDosDrive` to represent unmounted drives. - Enhanced `FloppyDiskDrive` and `FolderDrive` to implement `IDosPathContent` for file and directory operations. - Added `IsoDosPathContent` for ISO9660 CD image support. - Refactored `MemoryDrive` to support file operations and directory listing. - Removed obsolete `VirtualDrive` class. - Updated integration tests to cover new drive functionalities, including reading from mounted CD and floppy images. - Added assembly files for testing file reading from floppy and CD images. Signed-off-by: Maximilien Noal <noal.maximilien@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
It contains at least one definite runtime bug (recursive DosDriveManager.GetDriveStatuses) plus a couple of concrete correctness issues that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors DOS drive/path handling so non-folder-backed media (ISO9660 CD images and FAT floppies) can participate in DOS path resolution, file opens, and directory enumeration, and extends integration tests to validate reading from mounted CD/floppy images.
Changes:
- Introduces
IDosPathContent+DosContentEntryand updates drive types (FolderDrive,FloppyDiskDrive,MemoryDrive,CdRomDosDrive) to provide unified file/directory access. - Refactors path resolution logic by moving
DosPathResolverfunctionality intoDosDriveManager(partial), and updates consumers (INT 21h file manager, batch engine, MCP tooling). - Adds/updates integration tests and DOS .COM assembly fixtures for reading files from mounted ISO/floppy images, plus MSCDEX directory entry coverage.
File summaries
| File | Description |
|---|---|
| tests/Spice86.Tests/Resources/DosInt21Tests/read_floppy_image_file.asm | New DOS fixture to open/read a file from a mounted floppy image. |
| tests/Spice86.Tests/Resources/DosInt21Tests/read_cd_image_file.asm | New DOS fixture to open/read a file from a mounted CD image. |
| tests/Spice86.Tests/McpIntegrationContext.cs | Improves disposal safety for MCP integration resources. |
| tests/Spice86.Tests/Dos/SubstBatchCommandTests.cs | Updates expectations from removed VirtualDrive to FolderDrive. |
| tests/Spice86.Tests/Dos/MscdexDeviceRequestTests.cs | Adds MSCDEX “Get Directory Entry” tests and harness helper. |
| tests/Spice86.Tests/Dos/MountStatusBatchCommandTests.cs | Updates drive type assertions to FolderDrive. |
| tests/Spice86.Tests/Dos/DriveAbstractionTests.cs | Updates drive type assertions to FolderDrive. |
| tests/Spice86.Tests/Dos/DosTestHelpers.cs | Updates helper to construct the new DosDriveManager dependencies. |
| tests/Spice86.Tests/Dos/DosPathResolverTest.cs | Moves wildcard / SFN tests to DosDriveManager static APIs. |
| tests/Spice86.Tests/Dos/DosPathResolverIntegrationTests.cs | Uses DosDriveManager directly for path resolution integration tests. |
| tests/Spice86.Tests/Dos/DosInt21IntegrationTests.cs | Adds INT 21h tests for reading from mounted ISO/floppy images. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/VirtualDrive.cs | Removes obsolete VirtualDrive. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/MemoryDrive.cs | Makes MemoryDrive a DosDriveBase + IDosPathContent provider. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/IsoDosPathContent.cs | Adds ISO9660-backed IDosPathContent implementation for CD images. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/IDosPathContent.cs | New abstraction for DOS-relative file/directory access. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/FolderDrive.cs | New host-folder-backed drive with case-insensitive resolution and listing. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/FloppyDiskDrive.cs | Adds IDosPathContent implementation for FAT-backed floppy images. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/EmptyDosDrive.cs | New placeholder drive for valid letters with no mounted media. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/DosPathBuilder.cs | Points path constants to DosDriveManager after resolver refactor. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/DosFileEntryInfo.cs | Updates documentation reference from DosPathResolver to DosDriveManager. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/DosDriveBase.cs | Updates volume separator usage to DosDriveManager. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/DosContentEntry.cs | New cross-backend file/dir metadata record. |
| src/Spice86.Core/Emulator/OperatingSystem/Structures/CdRomDosDrive.cs | Adds IDosPathContent delegation for mounted CD image access. |
| src/Spice86.Core/Emulator/OperatingSystem/DosProcessManager.cs | Updates PATH initialization separators to DosDriveManager. |
| src/Spice86.Core/Emulator/OperatingSystem/DosPathResolver.cs | Converts resolver implementation into DosDriveManager (partial). |
| src/Spice86.Core/Emulator/OperatingSystem/DosFileManager.cs | Adds virtual content listing/open support for non-host-backed drives. |
| src/Spice86.Core/Emulator/OperatingSystem/DosDriveStatusProvider.cs | Updates status filtering/typing for new drive model. |
| src/Spice86.Core/Emulator/OperatingSystem/DosDriveManager.cs | Adds mount/image support, content map/list providers, and drive model changes. |
| src/Spice86.Core/Emulator/OperatingSystem/DosBatchExecutionEngine.CommandResolution.cs | Improves Z: file detection with current-drive-aware normalization. |
| src/Spice86.Core/Emulator/OperatingSystem/DosBatchExecutionEngine.CommandHandlers.cs | Updates drive typing and improves VOL drive argument handling. |
| src/Spice86.Core/Emulator/OperatingSystem/Dos.cs | Delegates mount/swap/content APIs to DosDriveManager; adjusts initialization order. |
| src/Spice86.Core/Emulator/OperatingSystem/Batch/HostPathResolver.cs | Updates host path resolution to use FolderDrive for mounted directories. |
| src/Spice86.Core/Emulator/Mcp/McpHttpHost.cs | Adds IAsyncDisposable and extends shutdown join timeout. |
| src/Spice86.Core/Emulator/Mcp/EmulatorMcpTools.cs | Updates MCP drive enumeration/type assumptions (FolderDrive). |
| src/Spice86.Core/Emulator/InterruptHandlers/Mscdex/MscdexErrorCode.cs | Adds a “FileNotFound” error code constant. |
| src/Spice86.Core/Emulator/InterruptHandlers/Mscdex/Mscdex.cs | Implements MSCDEX “Get Directory Entry” using ISO directory records. |
| src/Spice86.Core/Emulator/InterruptHandlers/Dos/DosInt21Handler.cs | Adds attribute/date handling for virtual-content entries lacking host paths. |
Review details
Suppressed comments (1)
src/Spice86.Core/Emulator/OperatingSystem/Structures/FolderDrive.cs:43
- Casting
System.IO.FileAttributesdirectly toDosFileAttributescan produce undefined DOS bits. Mask to the DOS/FAT attribute range before storing.
(DosFileAttributes)file.Attributes, file.CreationTimeUtc, file.FullName));
- Files reviewed: 37/39 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description of Changes
DosContentEntryto represent entries in DOS drives.DosDriveBaseto utilizeDosDriveManagerfor volume and path management.DosFileEntryInfoto reflect changes in path resolution.EmptyDosDriveto represent unmounted drives.FloppyDiskDriveandFolderDriveto implementIDosPathContentfor file and directory operations.IsoDosPathContentfor ISO9660 CD image support.MemoryDriveto support file operations and directory listing.VirtualDriveclass.Rationale behind Changes
MSCDEX had an unimplemented feature to find paths.
DosPathResolver was still all about folder based drives, and could not find files or folders on CDs or floppies.
MSCDEX enums were wrong.
MSCDEX documentation was not complete.
MSCDEX methods were made public for C# overrides of machine code.
Suggested Testing Steps
Full non-SingleStepTest suite: 2354 passed, 0 failed
Standalone MCP leak-sensitive test: passed
CD/floppy INT 21h tests: passed
MSCDEX directory tests: passed