From e39d0a2da0fb7316eaa573363002d1936728ff6b Mon Sep 17 00:00:00 2001
From: "Dylan M. Taylor" <dylan@dylanmtaylor.com>
Date: Tue, 3 Nov 2026 22:33:55 +0000
Subject: [PATCH] address review: reserved device names, fresh fifo test dir,
 stat before open in build.rs

---
 crates/engine/src/cmd/layout/saveas.rs   | 18 +++++++++++++++++-
 crates/engine/src/lisp/builtins/files.rs |  6 ++++--
 crates/fonts/build.rs                    |  4 ++++
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/crates/engine/src/cmd/layout/saveas.rs b/crates/engine/src/cmd/layout/saveas.rs
index bc36c0cb6..c7f8588fe 100644
--- a/crates/engine/src/cmd/layout/saveas.rs
+++ b/crates/engine/src/cmd/layout/saveas.rs
@@ -29,7 +29,11 @@ pub(super) fn default_path(s: &Session, layout: &str) -> String {
     // A layout name from a file may hold path separators or `..`; the suggestion is one file name
     // beside the drawing, so those become `_`.
     let name: String = layout.chars().map(|c| if matches!(c, '/' | '\\' | ':') || c.is_control() { '_' } else { c }).collect();
-    let file = format!("{name}.dwt");
+    // Windows reserves device names (`CON`, `NUL`, `COM1`...) with any extension, so prefix them.
+    let base = name.split('.').next().unwrap_or("").trim_end().to_ascii_uppercase();
+    let reserved = matches!(base.as_str(), "CON" | "PRN" | "AUX" | "NUL")
+        || (base.len() == 4 && (base.starts_with("COM") || base.starts_with("LPT")) && base.ends_with(|c: char| ('1'..='9').contains(&c)));
+    let file = if reserved { format!("_{name}.dwt") } else { format!("{name}.dwt") };
     match s.state().ok().and_then(|st| st.path.clone()) {
         Some(p) => std::path::Path::new(&p).with_file_name(&file).to_string_lossy().into_owned(),
         None => file,
@@ -152,4 +156,16 @@ mod default_path_tests {
         }
         assert_eq!(default_path(&s, "Layout1"), "Layout1.dwt");
     }
+
+    /// Windows device names are prefixed so the suggestion is never a device.
+    #[test]
+    fn reserved_device_names_are_prefixed() {
+        let s = Session::empty();
+        for name in ["CON", "nul", "Com1", "LPT9", "aux.x"] {
+            assert_eq!(default_path(&s, name), format!("_{name}.dwt"));
+        }
+        for name in ["COM0", "CONSOLE", "COM10"] {
+            assert_eq!(default_path(&s, name), format!("{name}.dwt"));
+        }
+    }
 }
diff --git a/crates/engine/src/lisp/builtins/files.rs b/crates/engine/src/lisp/builtins/files.rs
index ed63e9a41..b13312cbc 100644
--- a/crates/engine/src/lisp/builtins/files.rs
+++ b/crates/engine/src/lisp/builtins/files.rs
@@ -138,8 +138,10 @@ mod tests {
     /// Writing to a FIFO with no reader returns at once, and a regular file is still replaced.
     #[test]
     fn write_to_a_fifo_without_a_reader_does_not_block() {
-        let dir = std::env::temp_dir().join(format!("cadcraft-lispfifo-{}", std::process::id()));
-        std::fs::create_dir_all(&dir).unwrap();
+        // A fresh directory: `create_dir` fails if the name exists, so no stale pipe or planted symlink is reused.
+        let nanos = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos();
+        let dir = std::env::temp_dir().join(format!("cadcraft-lispfifo-{}-{nanos}", std::process::id()));
+        std::fs::create_dir(&dir).unwrap();
         let fifo = dir.join("pipe");
         assert!(std::process::Command::new("mkfifo").arg(&fifo).status().unwrap().success());
         let started = std::time::Instant::now();
diff --git a/crates/fonts/build.rs b/crates/fonts/build.rs
index 9063410ab..69f6c0486 100644
--- a/crates/fonts/build.rs
+++ b/crates/fonts/build.rs
@@ -82,6 +82,10 @@ const MAX_MANIFEST_BYTES: u64 = 1 << 20;
 /// Read the regular file `path` as text, at most [`MAX_MANIFEST_BYTES`].
 fn read_manifest(path: &std::path::Path) -> Result<String, String> {
     use std::io::Read as _;
+    // stat never blocks, but opening a FIFO with no writer does, so refuse non-regular files first.
+    if !std::fs::metadata(path).is_ok_and(|m| m.is_file()) {
+        return Err(format!("{}: not a regular file", path.display()));
+    }
     let file = std::fs::File::open(path).map_err(|e| format!("{}: {e}", path.display()))?;
     if !file.metadata().is_ok_and(|m| m.is_file()) {
         return Err(format!("{}: not a regular file", path.display()));