From 698c10c8c0380754e4552cb667a6ba30ee2a57c5 Mon Sep 17 00:00:00 2001 From: Julien Calixte Date: Sat, 11 Jul 2026 11:14:04 +0200 Subject: [PATCH] fix(sd): unlink target before f_rename to overwrite on FAT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FatFS's f_rename returns FR_EXIST on an existing destination — unlike POSIX rename(2) it won't replace it, so re-running the round-trip failed at rename. Unlink the target first (tolerating a missing one for the first save). This opens a crash window the real persistence module must close with *.tmp boot recovery (ADR-007). --- firmware/src/bin/sd_fat.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/firmware/src/bin/sd_fat.rs b/firmware/src/bin/sd_fat.rs index 089bb30..44f8a12 100644 --- a/firmware/src/bin/sd_fat.rs +++ b/firmware/src/bin/sd_fat.rs @@ -236,6 +236,19 @@ fn file_roundtrip() -> Result<()> { f.write_all(payload.as_bytes()).context("write tmp")?; f.sync_all().context("fsync tmp")?; // FatFS f_sync — flush before rename } + // FatFS's f_rename — unlike POSIX rename(2) — refuses to overwrite an + // existing destination and returns FR_EXIST (EEXIST). So the classic + // write-tmp → rename-over-target idiom needs an explicit unlink of the + // target first on FAT. That opens a crash window: the target is briefly + // gone while `tmp` holds the complete, fsync'd new content. The real + // persistence module must pair this with boot recovery — a lingering + // `*.tmp` means the last save didn't finish and should be promoted. See + // ADR-007. (Tolerate a missing target so the first save works too.) + match fs::remove_file(&path) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e).context("remove existing target before rename"), + } fs::rename(&tmp, &path).context("rename tmp -> final")?; let mut back = String::new();