This is a submission for DEV's Summer Bug Smash: Smash Stories powered by Sentry.
Written with the help of AI (Claude). The bug, the fix, the validation setup, and every claim below are mine, and were verified against the real codebase and a real full disk.
Someone lost a Magic: The Gathering decklist.
They were playing on Cockatrice — the open-source MTG client — with their decks on a drive that had quietly filled up while Oracle pushed an update in the background. They added a card, hit Ctrl+S, and Cockatrice said it saved. The debug log agreed:
[2026-05-28 22:31:42.031 I] Saved deck to "G:/cockatrice300/data/decks/edh-b2-gitrog-reanimate.cod" with format 1 - true
- true
. Success.
The file was 0 bytes. The deck was gone.
That was issue #6952, filed by Mekkiss. The steps to reproduce are four lines long and completely damning:
- Have a full disk.
- Open a deck on the full disk
- Add one card to it
- Save the deck (ctrl+s)
- Observe that the deck is now a 0 byte file.
The save path lived in Deck::saveToFile()
. Stripped down, it looked like this:
QFile file(fileName);
if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) {
qCWarning(DeckLog) << "Could not create or open file:" << fileName;
return std::nullopt;
}
bool success = false;
switch (fmt) { /* ... saveToFile_Native / saveToFile_Plain ... */ }
file.flush();
file.close();
qCInfo(DeckLog) << "Saved deck to " << fileName << "with format" << fmt << "-" << success;
There are three independent failures stacked on top of each other here, and you need all three to lose data:
1. WriteOnly truncates on open. The instant
open()
succeeds, the existing deck is 0 bytes. Not after a successful write — open()
still succeeds: truncating a file doesn't need free space. It frees space.2. The serializers always returned true.
saveToFile_Native()
and saveToFile_Plain()
write into the QTextStream
/ QIODevice
and return true
unconditionally. They never asked whether the bytes landed.3. The return value of flush() was discarded. This is the last place the truncation could still have been caught, and the result went straight into the void.
close()
after it can't help either — its failure was also ignored.So: file truncated, writes silently fail because there's no room, nobody checks, and the log cheerfully prints - true
. The user is told their deck is safe at the exact moment it stops existing.
While tracing the save path I checked the other places Deck
writes deck files. There were two more, and both used the same truncate-then-write-then-hope pattern.
updateLastLoadedTimestamp()
rewrites a deck to stamp it with a "last loaded" time — it runs on load, not save. Same QFile(fileName)
opened WriteOnly
, same always-true
serializer. On a full disk, merely opening a deck truncated it to 0 bytes and reported success. You could lose a decklist without ever pressing Ctrl+S.
convertToCockatriceFormat()
was the ugly one. It opened the destination .cod
file WriteOnly
, wrote the deck, and then — if result
was true, which it always was — deleted the original file:
file.close();
if (result) {
if (!QFile::remove(fileName)) { /* warn */ }
A failed write there doesn't leave you with a 0-byte file and a backup. It leaves you with a 0-byte file and no original. And because the function opened the destination before checking whether the source format was even convertible, the early-return branches ran with the file already truncated.
One user-visible bug on a full disk, and two more write paths queued up behind it waiting for the same conditions.
Qt has exactly the right tool for this and it has been sitting in QtCore since 5.1: QSaveFile
. It writes to a temporary file next to the target and only replaces the target — atomically — when you call commit()
and every byte has actually made it to disk. If anything fails, the original file is never touched.
The change is mostly deletion:
// Use QSaveFile so that a failed write (e.g. a full disk) leaves the existing deck untouched
// instead of truncating it to a 0-byte file. The target is only replaced once every byte has
// been flushed successfully in commit().
QSaveFile file(fileName);
if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) {
qCWarning(DeckLog) << "Could not create or open file:" << fileName;
return std::nullopt;
}
bool success = false;
switch (fmt) { /* ... */ }
if (!success) {
file.cancelWriting();
qCWarning(DeckLog) << "Failed to serialize deck for file:" << fileName;
return std::nullopt;
}
if (!file.commit()) {
qCWarning(DeckLog) << "Failed to save deck to " << fileName << ":" << file.errorString();
return std::nullopt;
}
qCInfo(DeckLog) << "Saved deck to " << fileName << "with format" << fmt;
Note what moved: the success log now happens after commit()
returns true, so it can no longer lie. The failure path logs file.errorString()
at warning level instead of printing - false
at info level and carrying on.
The same treatment went on all three write paths, plus one ordering fix in convertToCockatriceFormat()
— decide the format before opening anything, so an already-converted or unsupported deck can never be truncated and then deleted by a function that decided partway through it had nothing to do.
Net: +73 / −56 in one file. The success path behaves identically.
Here's the part I actually care about, because "I reasoned about it and it looks right" is how you ship a data-loss fix that doesn't work.
I couldn't reproduce this the way the reporter did. Cockatrice is a Qt desktop app, I wasn't going to build the whole GUI to test a file-I/O path, and — more importantly — I didn't want to fill up a real drive to find out.
So I made a disk that was genuinely, physically full:
diskpart
, formatted and mounted it.QFile
-
WriteOnly -
ignored-
flush()
pattern, and the new QSaveFile
commit()
pattern, both pointed at an existing non-empty file on the full volume.The result, against real Qt 6.8.1 QtCore:
old QFile path |
new QSaveFile path |
|
|---|---|---|
| Reported outcome | success | failure, with errorString()
|
| File on disk afterwards | 0 bytes |
original, intact |
That's a real ENOSPC
from a real filesystem, not a mock, not an injected error, not a #ifdef
. The old code lost the file and said it hadn't. The new code kept the file and said it couldn't save. Which is the entire point of the ticket.
The harness took maybe twenty minutes to set up and it is the only reason I'd put my name on the patch. If you're fixing a bug whose trigger is an environmental condition — full disk, no network, permission denied, clock skew — build the condition. Don't mock it. Mocks agree with whatever you already believe.
PR #6978 — merged into Cockatrice on 2026-06-09.
** - true in a log line is a claim, and claims need to be checked.** The log wasn't broken. It was faithfully reporting a variable that had been hardcoded to
true
three call frames down. Every layer was honest about the value it was handed; nobody ever checked whether the value meant anything.** WriteOnly is destructive before it's useful.** Any
open(WriteOnly)
on a path that already holds user data is a window where the data is gone and the replacement doesn't exist yet. If the write can fail — and it can always fail — that window is a data-loss bug waiting for the right Tuesday. QSaveFile
, write-temp-then-rename
, O_TMPFILE
linkat
: pick your platform's version, but pick one.When you find one instance of a pattern, grep for the pattern. The reported bug was one function. The pattern was three, and the scariest one fired on load. The user who filed #6952 would have eventually lost a deck just by opening it, and would never have connected that to a save bug.
Submitted for DEV's Summer Bug Smash — Smash Stories.