Ctrl+S said "Saved." The file was 0 bytes.

This is a submission for .

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.

The report

Someone lost a Magic: The Gathering decklist.

They were playing on , 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.

Three ways to be wrong at once

The save path lived in DeckLoader::saveToFile(). Stripped down, it looked like this:

QFile file(fileName);

if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) {

    qCWarning(DeckLoaderLog) << "Could not create or open file:" << fileName;

    return std::nullopt;

}



bool success = false;

switch (fmt) { /* ... saveToFile_Native / saveToFile_Plain ... */ }



file.flush();

file.close();



qCInfo(DeckLoaderLog) << "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 — at open time. The old deck is already destroyed before a single byte of the new one is written. On a full disk, 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.

It was worse than the report

While tracing the save path I checked the other places DeckLoader 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.

The fix

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(DeckLoaderLog) << "Could not create or open file:" << fileName;

    return std::nullopt;

}



bool success = false;

switch (fmt) { /* ... */ }



if (!success) {

    file.cancelWriting();

    qCWarning(DeckLoaderLog) << "Failed to serialize deck for file:" << fileName;

    return std::nullopt;

}



if (!file.commit()) {

    qCWarning(DeckLoaderLog) << "Failed to save deck to " << fileName << ":" << file.errorString();

    return std::nullopt;

}



qCInfo(DeckLoaderLog) << "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.

Proving it without a GUI

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:

1. Created a 16 MB VHD with diskpart, formatted and mounted it.


2. Filled it to exactly 0 bytes free.


3. Wrote a focused QtCore-only harness — no widgets, no Cockatrice — that reproduced the two code shapes side by side: the old 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:

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.

Shipped

— Smash Stories.