QFileSystemEngine/Unix: refactor rmpath()

Convert the path to QByteArray once up top, instead of using
QFile::encodeName() multiple times in the for-loop, and then use the
same QByteArray and truncate() it in each iteration.

Extend tst_qdir unittests.

Change-Id: I0d879e7f429db25879859acab074c2c197f41f6c
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Ahmad Samir 2024-06-10 19:04:19 +03:00
parent 889bdf1de4
commit 42bde675de
3 changed files with 31 additions and 11 deletions

View File

@ -1198,15 +1198,19 @@ bool QFileSystemEngine::rmdir(const QFileSystemEntry &entry)
bool QFileSystemEngine::rmpath(const QFileSystemEntry &entry)
{
const QString path = QDir::cleanPath(entry.filePath());
QByteArray path = QFile::encodeName(QDir::cleanPath(entry.filePath()));
Q_CHECK_FILE_NAME(path, false);
for (qsizetype oldslash = 0, slash = path.size(); slash > 0; oldslash = slash) {
const QByteArray chunk = QFile::encodeName(path.left(slash));
if (::rmdir(chunk.constData()) != 0)
return oldslash != 0;
if (::rmdir(path.constData()) != 0)
return false; // Only return false if `entry` couldn't be deleted
slash = path.lastIndexOf(QDir::separator(), oldslash - 1);
const char sep = QDir::separator().toLatin1();
qsizetype slash = path.lastIndexOf(sep);
// `slash > 0` because truncate(0) would make `path` empty
for (; slash > 0; slash = path.lastIndexOf(sep)) {
path.truncate(slash);
if (::rmdir(path.constData()) != 0)
break;
}
return true;

View File

@ -20,7 +20,6 @@ list(APPEND test_data_dirs "entrylist")
list(APPEND test_data_dirs "types")
list(APPEND test_data_dirs "tst_qdir.cpp")
set(test_data_files)
foreach(dir ${test_data_dirs})

View File

@ -364,10 +364,11 @@ void tst_QDir::mkdirRmdir_data()
const struct {
const char *name; // shall have a prefix added
const char *path; // relative
bool recurse;
bool recurse; // QDir::rmpath() vs. QDir::rmdir()
} cases[] = {
{ "plain", "testdir/one", false },
{ "recursive", "testdir/two/three/four", true },
{ "recursive-name-length-1", "a/b/c", true },
{ "with-..", "testdir/../testdir/three", false },
};
@ -383,6 +384,8 @@ void tst_QDir::mkdirRmdir()
QFETCH(QString, path);
QFETCH(bool, recurse);
QTest::ThrowOnFailEnabler thrower;
QDir dir;
dir.rmdir(path);
if (recurse)
@ -394,10 +397,24 @@ void tst_QDir::mkdirRmdir()
QFileInfo fi(path);
QVERIFY2(fi.exists() && fi.isDir(), msgDoesNotExist(path).constData());
if (recurse)
QVERIFY(dir.rmpath(path));
else
if (recurse) {
// Check that rmpath() removed all empty parent dirs
auto verifyRmPath = [&dir, &path](QLatin1StringView subdir) {
QFileInfo fi(QDir::currentPath() + subdir);
QVERIFY(fi.exists());
QVERIFY(dir.rmpath(path));
fi.refresh();
QVERIFY(!fi.exists());
};
if (path.contains("testdir/two/three/four"_L1))
verifyRmPath("/testdir/two"_L1);
else if (path.contains("a/b/c"_L1))
verifyRmPath("/a"_L1);
else
QVERIFY(dir.rmpath(path));
} else {
QVERIFY(dir.rmdir(path));
}
//make sure it really doesn't exist (ie that rmdir returns the right value)
fi.refresh();