moveToTrash/Unix: avoid creating too many QStorageInfo

QStorageInfo is great, but rather expensive, so this introduces a faster
check by stat()ing the source file and $HOME, to see if they are the
same device, saving us two or three QStorageInfo constructions. That is
a necessary condition: if they aren't the same device, we know rename()
into $HOME/.local/share/Trash will fail.

But it's not a sufficient condition: they need to be the same mount
point and that's something only QStorageInfo will give us. Strictly
speaking, the only way to be sure that you can rename() into the trash
path is to, well, attempt it (as usual, something for a later commit).

Change-Id: I9d43e5b91eb142d6945cfffd1786c474cac25083
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
This commit is contained in:
Thiago Macieira 2023-09-20 18:05:15 -07:00
parent de24134aa7
commit 6359e8b8bd

View File

@ -1194,7 +1194,7 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &, QFileSystemEnt
Implementing as per https://specifications.freedesktop.org/trash-spec/trashspec-1.0.html Implementing as per https://specifications.freedesktop.org/trash-spec/trashspec-1.0.html
*/ */
static QString freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemError &error) static auto freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemError &error)
{ {
auto makeTrashDir = [](const QDir &topDir, const QString &trashDir, QSystemError &error) { auto makeTrashDir = [](const QDir &topDir, const QString &trashDir, QSystemError &error) {
auto ownerPerms = QFileDevice::ReadOwner auto ownerPerms = QFileDevice::ReadOwner
@ -1212,19 +1212,33 @@ static QString freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemE
return targetDir; return targetDir;
return QString(); return QString();
}; };
struct R {
QString trashDir;
qsizetype volumePrefixLength = 0;
} r;
if (QFileSystemMetaData md; !QFileSystemEngine::fillMetaData(source, md, QFileSystemMetaData::ExistsAttribute) // first, check if they are in the same device
|| !md.exists()) { QString homePath = QFileSystemEngine::homePath();
error = QSystemError(ENOENT, QSystemError::StandardLibraryError); const QString sourcePath = source.filePath();
return QString(); QT_STATBUF sourceInfo, homeInfo;
if (QT_STAT(QFile::encodeName(sourcePath), &sourceInfo) != 0 ||
QT_STAT(QFile::encodeName(homePath), &homeInfo) != 0) {
error = QSystemError(errno, QSystemError::StandardLibraryError);
return r;
} }
QString trash;
const QString sourcePath = source.filePath();
const QStorageInfo sourceStorage(sourcePath); const QStorageInfo sourceStorage(sourcePath);
bool isHomeVolume = false;
if (sourceInfo.st_dev == homeInfo.st_dev) {
// being the same device is not enough for rename(): they must be the
// same mount, so we need QStorageInfo to compare
isHomeVolume = sourceStorage == QStorageInfo(QFileSystemEngine::homePath());
}
QString &trash = r.trashDir;
const QStorageInfo homeStorage(QDir::home()); const QStorageInfo homeStorage(QDir::home());
// We support trashing of files outside the users home partition // We support trashing of files outside the users home partition
if (sourceStorage != homeStorage) { if (!isHomeVolume) {
const auto dotTrash = "/.Trash"_L1; const auto dotTrash = "/.Trash"_L1;
QFileSystemEntry dotTrashDir(sourceStorage.rootPath() + dotTrash); QFileSystemEntry dotTrashDir(sourceStorage.rootPath() + dotTrash);
@ -1277,6 +1291,14 @@ static QString freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemE
const QString userTrashDir = dotTrash + u'-' + userID; const QString userTrashDir = dotTrash + u'-' + userID;
trash = makeTrashDir(QDir(sourceStorage.rootPath() + userTrashDir), QString(), error); trash = makeTrashDir(QDir(sourceStorage.rootPath() + userTrashDir), QString(), error);
} }
if (!trash.isEmpty()) {
r.volumePrefixLength = sourceStorage.rootPath().size();
if (r.volumePrefixLength == 1)
r.volumePrefixLength = 0; // isRoot
else
++r.volumePrefixLength; // to include the slash
}
} }
/* /*
"If both (1) and (2) fail [...], the implementation MUST either trash the "If both (1) and (2) fail [...], the implementation MUST either trash the
@ -1297,7 +1319,7 @@ static QString freeDesktopTrashLocation(const QFileSystemEntry &source, QSystemE
} }
} }
return trash; return r;
} }
//static //static
@ -1311,7 +1333,7 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source,
} }
return absoluteName(source); return absoluteName(source);
}(); }();
QString trashPath = freeDesktopTrashLocation(sourcePath, error); auto [trashPath, volumePrefixLength] = freeDesktopTrashLocation(sourcePath, error);
if (trashPath.isEmpty()) if (trashPath.isEmpty())
return false; return false;
QDir trashDir(trashPath); QDir trashDir(trashPath);
@ -1360,14 +1382,6 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source,
infoFile.setFileName(infoFileName); infoFile.setFileName(infoFileName);
} }
QString pathForInfo = sourcePath.filePath();
const QStorageInfo storageInfo(pathForInfo);
if (storageInfo.isValid() && storageInfo.rootPath() != rootPath() && storageInfo != QStorageInfo(QDir::home())) {
pathForInfo = std::move(pathForInfo).mid(storageInfo.rootPath().length());
if (pathForInfo.front() == u'/')
pathForInfo = pathForInfo.mid(1);
}
/* /*
We might fail to rename if source and target are on different file systems. We might fail to rename if source and target are on different file systems.
In that case, we don't try further, i.e. copying and removing the original In that case, we don't try further, i.e. copying and removing the original
@ -1382,7 +1396,7 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source,
QByteArray info = QByteArray info =
"[Trash Info]\n" "[Trash Info]\n"
"Path=" + QUrl::toPercentEncoding(pathForInfo, "/") + "\n" "Path=" + QUrl::toPercentEncoding(sourcePath.filePath().mid(volumePrefixLength), "/") + "\n"
"DeletionDate=" + QDateTime::currentDateTime().toString(Qt::ISODate).toUtf8() "DeletionDate=" + QDateTime::currentDateTime().toString(Qt::ISODate).toUtf8()
+ "\n"; + "\n";
infoFile.write(info); infoFile.write(info);