diff --git a/src/filesys.rs b/src/filesys.rs index 3b61b0d..a52ef9b 100644 --- a/src/filesys.rs +++ b/src/filesys.rs @@ -1006,7 +1006,7 @@ impl FilesysUnit { if path.is_dir() { return (DOSFALSE, ERROR_OBJECT_WRONG_TYPE); } - match std::fs::File::open(&path) { + match open_existing(&path) { Ok(f) => { self.next_file_key += 1; let key = self.next_file_key; @@ -1054,7 +1054,7 @@ impl FilesysUnit { if path.is_dir() { return (DOSFALSE, ERROR_OBJECT_WRONG_TYPE); } - match std::fs::File::open(&path) { + match open_existing(&path) { Ok(f) => { self.next_file_key += 1; let key = self.next_file_key; @@ -1983,6 +1983,22 @@ fn civil_from_days(z: i64) -> (i64, i64, i64) { (if m <= 2 { y + 1 } else { y }, m, d) } +/// Open an existing file writable, the way AmigaDOS hands out a file handle. +/// When the host denies write only because the file or its filesystem is +/// read-only, fall back to a read-only handle, so a later write fails as it +/// would on a protected Amiga file. Any other error propagates unchanged. +fn open_existing(path: &Path) -> std::io::Result { + use std::io::ErrorKind as K; + std::fs::OpenOptions::new() + .read(true) + .write(true) + .open(path) + .or_else(|e| match e.kind() { + K::PermissionDenied | K::ReadOnlyFilesystem => std::fs::File::open(path), + _ => Err(e), + }) +} + /// Map a host I/O error onto the AmigaDOS error the guest expects, so a full /// disk says "disk full" rather than a generic failure. fn host_error(e: &std::io::Error) -> u32 { @@ -2248,6 +2264,81 @@ mod tests { } } + /// A single mounted HOSTFS unit backed by a temp dir, over a flat guest + /// RAM, for driving `handle_packet` without a machine. Fixed guest + /// addresses for the objects a packet references; the temp dir is removed + /// on drop. + struct FsTester { + root: PathBuf, + board: FilesysBoard, + bus: RamBus, + guest_op: Option, + } + + impl FsTester { + const BOARD_BASE: u32 = 0x1_0000; + const PORT: u32 = 0x3000; + const PKT: u32 = 0x2000; + const DN: u32 = 0x1000; + const FH: u32 = 0x4000; + const NAME: u32 = 0x5000; + const BUF: u32 = 0x6000; + + /// Mount `tag` and send the startup packet, so the unit is wired. + fn mounted(tag: &str) -> Self { + let root = std::env::temp_dir().join(format!("clfs-{tag}-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + let mut board = FilesysBoard::default(); + board.set_mounts(vec![MountSpec { + path: root.clone(), + volume: "Test".into(), + boot_pri: -128, + readonly: false, + }]); + let mut fs = Self { + root, + board, + bus: RamBus(vec![0u8; 0x2_0000]), + guest_op: None, + }; + fs.send(0, [0, 0, Self::DN >> 2]); + fs + } + + /// Post one packet of type `ty` with three args; returns (Res1, Res2). + fn send(&mut self, ty: i32, args: [u32; 3]) -> (u32, u32) { + self.bus.write_long(Self::PKT + 8, ty as u32); + for (i, a) in args.iter().enumerate() { + self.bus.write_long(Self::PKT + 20 + 4 * i as u32, *a); + } + self.board.units[0].handle_packet( + &mut self.bus, + Self::BOARD_BASE, + Self::PORT, + Self::PKT, + &mut self.guest_op, + ) + } + + /// Lay a BSTR file name at NAME for the next packet to reference. + fn set_name(&mut self, s: &[u8]) { + self.bus.0[Self::NAME as usize] = s.len() as u8; + self.bus.0[Self::NAME as usize + 1..Self::NAME as usize + 1 + s.len()] + .copy_from_slice(s); + } + + /// Write `bytes` into the shared buffer at BUF. + fn set_buf(&mut self, bytes: &[u8]) { + self.bus.0[Self::BUF as usize..Self::BUF as usize + bytes.len()].copy_from_slice(bytes); + } + } + + impl Drop for FsTester { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.root); + } + } + /// The whole MMIO surface, end to end through [`ZorroDevice`]: the /// DIAG_DOORBELL captures the board base, the PORT write introduces the /// handler, and the packet doorbell handles a startup packet placed in @@ -2529,114 +2620,87 @@ mod tests { std::fs::remove_dir_all(&root).unwrap(); } + /// A handle on an existing file is writable, whichever way it was opened. + /// This is Workbench's Snapshot: icon.library takes a shared lock on the + /// .info, OpenFromLock()s it, reads the header, and writes it back through + /// the same handle. Opening the host file read-only failed that write. + #[test] + fn handles_on_existing_files_are_writable() { + let mut fs = FsTester::mounted("rw"); + fs.set_buf(b"new"); + + // OpenFromLock() on a shared lock, then Open(MODE_OLDFILE), which is + // read/write on the Amiga as well. + for (file, from_lock) in [("a.info", true), ("b.info", false)] { + std::fs::write(fs.root.join(file), b"old").unwrap(); + fs.set_name(file.as_bytes()); + let open = if from_lock { + let (lock, _) = + fs.send(ACTION_LOCATE_OBJECT, [0, FsTester::NAME >> 2, ACCESS_READ]); + fs.send(ACTION_FH_FROM_LOCK, [FsTester::FH >> 2, lock, 0]) + } else { + fs.send( + ACTION_FINDINPUT, + [FsTester::FH >> 2, 0, FsTester::NAME >> 2], + ) + }; + assert_eq!(open, (DOSTRUE, 0), "open {file}"); + let cookie = fs.bus.read_long(FsTester::FH + FILEHANDLE_ARG1); + assert_eq!( + fs.send(ACTION_WRITE, [cookie, FsTester::BUF, 3]), + (3, 0), + "write {file}" + ); + fs.send(ACTION_END, [cookie, 0, 0]); + assert_eq!(std::fs::read(fs.root.join(file)).unwrap(), b"new"); + } + } + /// Seek offsets are signed LONGs: a backward relative seek (negative /// Arg2) must move the position, not zero-extend into a huge forward /// seek. This is iffparse's PopChunk pattern -- stream chunks with /// placeholder lengths, then Seek(-n, OFFSET_CURRENT/OFFSET_END) back /// and patch them -- which P96Prefs saves rely on. #[test] - fn seek_accepts_negative_offsets_like_iffparse() { - let root = std::env::temp_dir().join(format!("clfs-seek-{}", std::process::id())); - std::fs::create_dir_all(&root).unwrap(); - - let mut hle = FilesysBoard::default(); - hle.set_mounts(vec![MountSpec { - path: root.clone(), - volume: "Test".into(), - boot_pri: -128, - readonly: false, - }]); - let mut bus = RamBus(vec![0u8; 0x2_0000]); - let (board_base, port, pkt, dn, fh, name, buf) = ( - 0x1_0000u32, - 0x3000u32, - 0x2000u32, - 0x1000u32, - 0x4000u32, - 0x5000u32, - 0x6000u32, - ); - let mut guest_op = None; - let send = |bus: &mut RamBus, - unit: &mut FilesysUnit, - guest_op: &mut Option, - ty: i32, - args: [u32; 3]| { - bus.write_long(pkt + 8, ty as u32); - for (i, a) in args.iter().enumerate() { - bus.write_long(pkt + 20 + 4 * i as u32, *a); - } - unit.handle_packet(bus, board_base, port, pkt, guest_op) - }; + fn seek_accepts_negative_offsets() { + let mut fs = FsTester::mounted("seek"); - // Startup packet wires the unit; then Open("iff.bin", MODE_NEWFILE). - let unit = &mut hle.units[0]; - send(&mut bus, unit, &mut guest_op, 0, [0, 0, dn >> 2]); - bus.0[name as usize] = 7; - bus.0[name as usize + 1..name as usize + 8].copy_from_slice(b"iff.bin"); - let (res1, res2) = send( - &mut bus, - unit, - &mut guest_op, - ACTION_FINDOUTPUT, - [fh >> 2, 0, name >> 2], + // Open("iff.bin", MODE_NEWFILE). + fs.set_name(b"iff.bin"); + assert_eq!( + fs.send( + ACTION_FINDOUTPUT, + [FsTester::FH >> 2, 0, FsTester::NAME >> 2] + ), + (DOSTRUE, 0) ); - assert_eq!((res1, res2), (DOSTRUE, 0)); - let cookie = bus.read_long(fh + FILEHANDLE_ARG1); + let cookie = fs.bus.read_long(FsTester::FH + FILEHANDLE_ARG1); // Stream 24 bytes with two 0xFFFFFFFF length placeholders. - bus.0[buf as usize..buf as usize + 24] - .copy_from_slice(b"FORM\xff\xff\xff\xffP96SANNO\xff\xff\xff\xffp96p"); - let (res1, _) = send( - &mut bus, - unit, - &mut guest_op, - ACTION_WRITE, - [cookie, buf, 24], - ); - assert_eq!(res1, 24); + fs.set_buf(b"FORM\xff\xff\xff\xffP96SANNO\xff\xff\xff\xffp96p"); + assert_eq!(fs.send(ACTION_WRITE, [cookie, FsTester::BUF, 24]).0, 24); // Patch the ANNO length: back 8 from the current position (24 -> 16). - let (res1, res2) = send( - &mut bus, - unit, - &mut guest_op, - ACTION_SEEK, - [cookie, (-8i32) as u32, OFFSET_CURRENT as u32], - ); - assert_eq!((res1, res2), (24, 0), "backward OFFSET_CURRENT seek failed"); - bus.0[buf as usize..buf as usize + 4].copy_from_slice(&4u32.to_be_bytes()); - send( - &mut bus, - unit, - &mut guest_op, - ACTION_WRITE, - [cookie, buf, 4], + assert_eq!( + fs.send(ACTION_SEEK, [cookie, (-8i32) as u32, OFFSET_CURRENT as u32]), + (24, 0), + "backward OFFSET_CURRENT seek failed" ); + fs.set_buf(&4u32.to_be_bytes()); + fs.send(ACTION_WRITE, [cookie, FsTester::BUF, 4]); // Patch the FORM length: 20 back from the end (24 -> 4). - let (res1, res2) = send( - &mut bus, - unit, - &mut guest_op, - ACTION_SEEK, - [cookie, (-20i32) as u32, OFFSET_END as u32], - ); - assert_eq!((res1, res2), (20, 0), "backward OFFSET_END seek failed"); - bus.0[buf as usize..buf as usize + 4].copy_from_slice(&16u32.to_be_bytes()); - send( - &mut bus, - unit, - &mut guest_op, - ACTION_WRITE, - [cookie, buf, 4], + assert_eq!( + fs.send(ACTION_SEEK, [cookie, (-20i32) as u32, OFFSET_END as u32]), + (20, 0), + "backward OFFSET_END seek failed" ); + fs.set_buf(&16u32.to_be_bytes()); + fs.send(ACTION_WRITE, [cookie, FsTester::BUF, 4]); - send(&mut bus, unit, &mut guest_op, ACTION_END, [cookie, 0, 0]); - let on_disk = std::fs::read(root.join("iff.bin")).unwrap(); + fs.send(ACTION_END, [cookie, 0, 0]); + let on_disk = std::fs::read(fs.root.join("iff.bin")).unwrap(); assert_eq!(on_disk, b"FORM\x00\x00\x00\x10P96SANNO\x00\x00\x00\x04p96p"); - - std::fs::remove_dir_all(&root).unwrap(); } /// Latin-1 <-> UTF-8 mapping matches amiberry: a guest name with high bytes