File Save Operation Should Be Atomic to Prevent Data Loss and Corruption

[KjellMorgenstern]

Current Behaviour

When saving changes to a file, there is a moment when the original file content is deleted, before it is then rewritten. This is risky.

Observed scenarios:

• There are conditions in the code that could stop writing the new content. Namely, if the content of the .ino code changed, and while the code view is open, Ctrl-S is pressed to save the file, and the dialog asking for the name of the .ino file is triggered, and then cancelled by the user, the write operation will not happen, although the original fzz was already overwritten.
• Remote file systems will perform bad because of the two successive file rewrites. In some network file systems, this could trigger bugs, leaving the file corrupted.

Speculative scenarios (not verified)

• Before overwriting the original file, we should write the new data to a temporary location. Once that succeeded, we can copy the new content over the previous one. Since that is a simple file operation, there are few chances to go wrong, and we would still have the data in the temporary file if it does. However, since we already overwrote the original file in the beginning, any error in writing the temp file would result in data loss.
• 3rd party tools like virus scanners could block write operations, so these should be atomic, to avoid being interrupted in an inconsistent state.
• If a disk is full, setting the file to zero size might work, and then trying to write it again (or writing to a temp file) might fail.

Build: All versions

Operating System: All

Steps to reproduce: Place a breakpoint in mainwindow_export.cpp:

bool MainWindow::saveAsAux(const QString & fileName) {
    QFile file(fileName);
    if (!file.open(QFile::WriteOnly | QFile::Text)) {   // <-- break here
        QMessageBox::warning(this, tr("Fritzing"),
                             tr("Cannot write file %1:\n%2.")
                             .arg(fileName, file.errorString()));
        return false;
    }

    file.close();

    setReadOnly(false);

Observe that the file size is zero after the file.close() was executed.

Expected Behaviour

The writability check must be non-destructive.

[KjellMorgenstern]

Fixed with abe60fe6c