From 675d6fd5588c65928224756ada41a9c294db608a Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@intel.com>
Date: Thu, 23 Jan 2025 20:24:02 -0800
Subject: [PATCH] QFile::rename: merge the block copy code with copy()

This greatly simplifies the code and removes the duplication of the
block copy content. For example, it was missing both the file engine-
level copy() and cloneTo() optimizations.

Now a rename across filesystems on Linux will use the OS copy system
calls:
ioctl(5, BTRFS_IOC_CLONE or FICLONE, 4) = -1 EXDEV (Invalid cross-device link)
copy_file_range(4, [0], 5, [0], 9223372036854775807, 0) = -1 EXDEV (Invalid cross-device link)
sendfile(5, 4, NULL, 2147479552)        = 1111
sendfile(5, 4, NULL, 2147479552)        = 0
statx(4, "", AT_STATX_SYNC_AS_STAT|AT_NO_AUTOMOUNT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=1111, ...}) = 0
access("x", R_OK)                       = 0
access("x", W_OK)                       = 0
access("x", X_OK)                       = -1 EACCES (Permission denied)
fchmod(5, 0644)                         = 0
close(4)                                = 0
lseek(5, 0, SEEK_SET)                   = 0
fdatasync(5)                            = 0
linkat(AT_FDCWD, "/proc/self/fd/5", AT_FDCWD, "/tmp/x", AT_SYMLINK_FOLLOW) = 0
close(5)                                = 0
unlink("x")                             = 0

As a side-effect, we won't be able to rename files across volumes
without QSaveFile.

Change-Id: Ic864f4833cb21de1e9f7fffd811c5d59a10c9eb7
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
---
 src/corelib/io/qfile.cpp                  | 56 ++++++++---------------
 tests/auto/corelib/io/qfile/tst_qfile.cpp |  5 ++
 2 files changed, 25 insertions(+), 36 deletions(-)

diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp
index 263806c542d..c645e57bdb6 100644
--- a/src/corelib/io/qfile.cpp
+++ b/src/corelib/io/qfile.cpp
@@ -667,46 +667,30 @@ QFile::rename(const QString &newName)
             return false;
         }
 
-        QFile out(newName);
-        if (open(QIODevice::ReadOnly | QIODevice::Unbuffered)) {
-            if (out.open(QIODevice::WriteOnly | QIODevice::Truncate | QIODevice::Unbuffered)) {
-                bool error = false;
-                char block[4096];
-                qint64 bytes;
-                while ((bytes = read(block, sizeof(block))) > 0) {
-                    if (bytes != out.write(block, bytes)) {
-                        d->setError(QFile::RenameError, out.errorString());
-                        error = true;
-                        break;
-                    }
-                }
-                if (bytes == -1) {
-                    d->setError(QFile::RenameError, errorString());
-                    error = true;
-                }
-                if (!error) {
-                    if (!remove()) {
-                        d->setError(QFile::RenameError, tr("Cannot remove source file"));
-                        error = true;
-                    }
-                }
-                if (error) {
-                    out.remove();
-                } else {
-                    d->fileEngine->setFileName(newName);
-                    setPermissions(permissions());
-                    unsetError();
-                    setFileName(newName);
-                }
-                close();
-                return !error;
+#if QT_CONFIG(temporaryfile)
+        // copy the file to the destination first
+        if (d->copy(newName)) {
+            // succeeded, remove the original
+            if (!remove()) {
+                d->setError(QFile::RenameError, tr("Cannot remove source file: %1").arg(errorString()));
+                QFile out(newName);
+                // set it back to writable so we can delete it
+                out.setPermissions(ReadUser | WriteUser);
+                out.remove(newName);
+                return false;
             }
-            close();
-            d->setError(QFile::RenameError,
-                        tr("Cannot open destination file: %1").arg(out.errorString()));
+            d->fileEngine->setFileName(newName);
+            unsetError();
+            setFileName(newName);
+            return true;
         } else {
+            // change the error type but keep the string
             d->setError(QFile::RenameError, errorString());
         }
+#else
+        // copy the error from the engine rename() above
+        d->setError(QFile::RenameError, d->fileEngine->errorString());
+#endif
     }
     return false;
 }
diff --git a/tests/auto/corelib/io/qfile/tst_qfile.cpp b/tests/auto/corelib/io/qfile/tst_qfile.cpp
index 306dda14690..184853b28d7 100644
--- a/tests/auto/corelib/io/qfile/tst_qfile.cpp
+++ b/tests/auto/corelib/io/qfile/tst_qfile.cpp
@@ -3026,6 +3026,11 @@ void tst_QFile::renameFallback()
     QFile::remove("file-rename-destination.txt");
 
     QVERIFY(!file.rename("file-rename-destination.txt"));
+#ifdef Q_OS_WIN
+    // wait for the file to disappear
+    QTRY_VERIFY_WITH_TIMEOUT(!QFile::exists("file-rename-destination.txt"),
+                             std::chrono::seconds(1));
+#endif
     QVERIFY(!QFile::exists("file-rename-destination.txt"));
     QVERIFY(!file.isOpen());
 }