From 83f7125ac866ed5a2aaace56ffc866f21f9730b4 Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Thu, 14 Dec 2023 18:26:33 +0200 Subject: [PATCH] QFileSystemIterator/Unix: use QStringDecoder The end result is a QFileInfo constructed from the QFileSystemEntry; QFileSystemEntry's API will at some point convert the NativePath (QBA on Unix) to a QString, e.g. when it calls findLastSeparator(). So instead of converting the parent dir path from Utf8 to Utf16 multiple times, store the dir path as a QString once in the constructor. Use QStringDecoder to convert the dirent->d_name to Utf16 and check the decoder's hasError(); which as Thiago pointed out, is more efficient than using isValidUtf8(), throwing away the results, then converting to unicode anyway later on in QDirIteratorPrivate::advance(). (Ironically) `QFileSystemEntry::NativePath nativePath` is still used on Windows. Change-Id: Icd2b130103640b502862d210d64926b99c499a01 Reviewed-by: Thiago Macieira --- src/corelib/io/qfilesystemiterator_p.h | 9 ++++-- src/corelib/io/qfilesystemiterator_unix.cpp | 34 +++++++++++---------- src/corelib/io/qfilesystemiterator_win.cpp | 4 +-- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/corelib/io/qfilesystemiterator_p.h b/src/corelib/io/qfilesystemiterator_p.h index 6638016a41..5687da115e 100644 --- a/src/corelib/io/qfilesystemiterator_p.h +++ b/src/corelib/io/qfilesystemiterator_p.h @@ -26,6 +26,10 @@ #include #include +#if !defined(Q_OS_WIN) +#include +#endif + #include QT_BEGIN_NAMESPACE @@ -39,11 +43,11 @@ public: bool advance(QFileSystemEntry &fileEntry, QFileSystemMetaData &metaData); private: - QFileSystemEntry::NativePath nativePath; + QString dirPath; // Platform-specific data #if defined(Q_OS_WIN) - QString dirPath; + QFileSystemEntry::NativePath nativePath; HANDLE findFileHandle; QStringList uncShares; bool uncFallback; @@ -58,6 +62,7 @@ private: QT_DIRENT *dirEntry = nullptr; int lastError = 0; + QStringDecoder toUtf16; #endif Q_DISABLE_COPY_MOVE(QFileSystemIterator) diff --git a/src/corelib/io/qfilesystemiterator_unix.cpp b/src/corelib/io/qfilesystemiterator_unix.cpp index cd8d9400cc..30b1a005a9 100644 --- a/src/corelib/io/qfilesystemiterator_unix.cpp +++ b/src/corelib/io/qfilesystemiterator_unix.cpp @@ -4,10 +4,10 @@ #include "qplatformdefs.h" #include "qfilesystemiterator_p.h" -#include - #ifndef QT_NO_FILESYSTEMITERATOR +#include + #include #include @@ -15,20 +15,13 @@ QT_BEGIN_NAMESPACE -static bool checkNameDecodable(const char *d_name, qsizetype len) -{ - // This function is called in a loop from advance() below, but the loop is - // usually run only once. - - return QUtf8::isValidUtf8(QByteArrayView(d_name, len)).isValidUtf8; -} - /* Native filesystem iterator, which uses ::opendir()/readdir()/dirent from the system libraries to iterate over the directory represented by \a entry. */ QFileSystemIterator::QFileSystemIterator(const QFileSystemEntry &entry, QDir::Filters filters) - : nativePath(entry.nativeFilePath()) + : dirPath(entry.filePath()), + toUtf16(QStringDecoder::Utf8) { Q_UNUSED(filters); @@ -36,8 +29,8 @@ QFileSystemIterator::QFileSystemIterator(const QFileSystemEntry &entry, QDir::Fi if (!dir) { lastError = errno; } else { - if (!nativePath.endsWith('/')) - nativePath.append('/'); + if (!dirPath.endsWith(u'/')) + dirPath.append(u'/'); } } @@ -58,9 +51,18 @@ bool QFileSystemIterator::advance(QFileSystemEntry &fileEntry, QFileSystemMetaDa dirEntry = QT_READDIR(dir.get()); if (dirEntry) { - qsizetype len = strlen(dirEntry->d_name); - if (checkNameDecodable(dirEntry->d_name, len)) { - fileEntry = QFileSystemEntry(nativePath + QByteArray(dirEntry->d_name, len), QFileSystemEntry::FromNativePath()); + // POSIX allows readdir() to return a file name in struct dirent that + // extends past the end of the d_name array (it's a char[1] array on QNX, for + // example). Therefore, we *must* call strlen() on it to get the actual length + // of the file name. See: + // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/dirent.h.html#tag_13_07_05 + QByteArrayView name(dirEntry->d_name, strlen(dirEntry->d_name)); + // name.size() is sufficient here, see QUtf8::convertToUnicode() for details + QVarLengthArray buffer(name.size()); + auto *end = toUtf16.appendToBuffer(buffer.data(), name); + buffer.resize(end - buffer.constData()); + if (!toUtf16.hasError()) { + fileEntry = {dirPath + QStringView(buffer), QFileSystemEntry::FromInternalPath()}; metaData.fillFromDirEnt(*dirEntry); return true; } else { diff --git a/src/corelib/io/qfilesystemiterator_win.cpp b/src/corelib/io/qfilesystemiterator_win.cpp index 30c39f6b4d..7644a1a078 100644 --- a/src/corelib/io/qfilesystemiterator_win.cpp +++ b/src/corelib/io/qfilesystemiterator_win.cpp @@ -15,8 +15,8 @@ using namespace Qt::StringLiterals; bool done = true; QFileSystemIterator::QFileSystemIterator(const QFileSystemEntry &entry, QDir::Filters filters) - : nativePath(entry.nativeFilePath()) - , dirPath(entry.filePath()) + : dirPath(entry.filePath()) + , nativePath(entry.nativeFilePath()) , findFileHandle(INVALID_HANDLE_VALUE) , uncFallback(false) , uncShareIndex(0)