QFSFileEngine: make rename() on Unix not overwrite

The rename(2) system call overwrites, so instead of using it, we try to
use the link/unlink pair. This works for regular cases, but can fail if
trying to change case in case-insensitive filesystems, if we're
operating on a non-Unix filesystem (FAT) or, on Linux, if the file
doesn't belong to the calling user (BSDs permit this). For those cases,
we fall back to rename(2).

That means there's a race condition if a new file is created there. But
we at least reduce the likelihood of that happening for regular files.

Change-Id: I1eba2b016de74620bfc8fffd14ccb38fd929e5aa
Reviewed-by: David Faure <david.faure@kdab.com>

src/corelib/io/qfile.cpp

@@ -566,18 +566,18 @@ QFile::rename(const QString &newName)
         d->setError(QFile::RenameError, tr("Source file does not exist."));
         return false;
     }
+
     // If the file exists and it is a case-changing rename ("foo" -> "Foo"),
     // compare Ids to make sure it really is a different file.
     // Note: this does not take file engines into account.
+    bool changingCase = false;
     QByteArray targetId = QFileSystemEngine::id(QFileSystemEntry(newName));
     if (!targetId.isNull()) {
         QByteArray fileId = d->fileEngine ?
                     d->fileEngine->id() :
                     QFileSystemEngine::id(QFileSystemEntry(d->fileName));
-        if (fileId != targetId || d->fileName.compare(newName, Qt::CaseInsensitive)) {
+        changingCase = (fileId == targetId && d->fileName.compare(newName, Qt::CaseInsensitive) == 0);
-            // ### Race condition. If a file is moved in after this, it /will/ be
+        if (!changingCase) {
-            // overwritten. On Unix, the proper solution is to use hardlinks:
-            // return ::link(old, new) && ::remove(old);
             d->setError(QFile::RenameError, tr("Destination file exists"));
             return false;
         }
@@ -593,7 +593,7 @@ QFile::rename(const QString &newName)
             return false;
         }
         tempFile.close();
-        if (!d->engine()->rename(tempFile.fileName())) {
+        if (!d->engine()->renameOverwrite(tempFile.fileName())) {
             d->setError(QFile::RenameError, tr("Error while renaming."));
             return false;
         }
@@ -616,7 +616,7 @@ QFile::rename(const QString &newName)
     unsetError();
     close();
     if(error() == QFile::NoError) {
-        if (d->engine()->rename(newName)) {
+        if (changingCase ? d->engine()->renameOverwrite(newName) : d->engine()->rename(newName)) {
             unsetError();
             // engine was able to handle the new name so we just reset it
             d->fileEngine->setFileName(newName);

src/corelib/io/qfilesystemengine_p.h

@@ -121,6 +121,7 @@ public:
 
     static bool copyFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error);
     static bool renameFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error);
+    static bool renameOverwriteFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error);
     static bool removeFile(const QFileSystemEntry &entry, QSystemError &error);
 
     static bool setPermissions(const QFileSystemEntry &entry, QFile::Permissions permissions, QSystemError &error,

src/corelib/io/qfilesystemengine_unix.cpp

@@ -650,6 +650,50 @@ bool QFileSystemEngine::copyFile(const QFileSystemEntry &source, const QFileSyst
 
 //static
 bool QFileSystemEngine::renameFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error)
+{
+    QFileSystemEntry::NativePath srcPath = source.nativeFilePath();
+    QFileSystemEntry::NativePath tgtPath = target.nativeFilePath();
+    if (::link(srcPath, tgtPath) == 0) {
+        if (::unlink(srcPath) == 0)
+            return true;
+
+        // if we managed to link but can't unlink the source, it's likely
+        // it's in a directory we don't have write access to; fail the
+        // renaming instead
+        int savedErrno = errno;
+
+        // this could fail too, but there's nothing we can do about it now
+        ::unlink(tgtPath);
+
+        error = QSystemError(savedErrno, QSystemError::StandardLibraryError);
+        return false;
+    }
+
+    switch (errno) {
+    case EACCES:
+    case EEXIST:
+    case ENAMETOOLONG:
+    case ENOENT:
+    case ENOTDIR:
+    case EROFS:
+    case EXDEV:
+        // accept the error from link(2) (especially EEXIST) and don't retry
+        break;
+
+    default:
+        // fall back to rename()
+        // ### Race condition. If a file is moved in after this, it /will/ be
+        // overwritten.
+        if (::rename(srcPath, tgtPath) == 0)
+            return true;
+    }
+
+    error = QSystemError(errno, QSystemError::StandardLibraryError);
+    return false;
+}
+
+//static
+bool QFileSystemEngine::renameOverwriteFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error)
 {
     if (::rename(source.nativeFilePath().constData(), target.nativeFilePath().constData()) == 0)
         return true;

src/corelib/io/qfilesystemengine_win.cpp

@@ -1246,6 +1246,17 @@ bool QFileSystemEngine::renameFile(const QFileSystemEntry &source, const QFileSy
     return ret;
 }
 
+//static
+bool QFileSystemEngine::renameOverwriteFile(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error)
+{
+    bool ret = ::MoveFileEx(reinterpret_cast<const wchar_t *>(source.nativeFilePath().utf16()),
+                            reinterpret_cast<const wchar_t *>(target.nativeFilePath().utf16()),
+                            MOVEFILE_REPLACE_EXISTING) != 0;
+    if (!ret)
+        error = QSystemError(::GetLastError(), QSystemError::NativeError);
+    return ret;
+}
+
 //static
 bool QFileSystemEngine::removeFile(const QFileSystemEntry &entry, QSystemError &error)
 {

src/corelib/io/qfsfileengine_unix.cpp

@@ -362,8 +362,14 @@ bool QFSFileEngine::copy(const QString &newName)
 
 bool QFSFileEngine::renameOverwrite(const QString &newName)
 {
-    // On Unix, rename() overwrites.
+    Q_D(QFSFileEngine);
-    return rename(newName);
+    QSystemError error;
+    bool ret = QFileSystemEngine::renameOverwriteFile(d->fileEntry, QFileSystemEntry(newName), error);
+
+    if (!ret)
+        setError(QFile::RenameError, error.toString());
+
+    return ret;
 }
 
 bool QFSFileEngine::rename(const QString &newName)

src/corelib/io/qfsfileengine_win.cpp

@@ -507,11 +507,10 @@ bool QFSFileEngine::rename(const QString &newName)
 bool QFSFileEngine::renameOverwrite(const QString &newName)
 {
     Q_D(QFSFileEngine);
-    bool ret = ::MoveFileEx((wchar_t*)d->fileEntry.nativeFilePath().utf16(),
+    QSystemError error;
-                            (wchar_t*)QFileSystemEntry(newName).nativeFilePath().utf16(),
+    bool ret = QFileSystemEngine::renameOverwriteFile(d->fileEntry, QFileSystemEntry(newName), error);
-                            MOVEFILE_REPLACE_EXISTING) != 0;
     if (!ret)
-        setError(QFile::RenameError, QSystemError(::GetLastError(), QSystemError::NativeError).toString());
+        setError(QFile::RenameError, error.toString());
     return ret;
 }