Fixes QDir not reentrant

qt_cmp_si_sort_flags could be read and written from different threads.

Use qStableSort with functor instead of libc's quicksort.

Found with helgrind on kdevelop

Reviewed-by: Marius Storm-Olsen

1 files changed, 18 insertions, 29 deletions

diff --git a/src/corelib/io/qdir.cpp b/src/corelib/io/qdir.cpp
index 6d75c59..b7861ba 100644
--- a/src/corelib/io/qdir.cpp
+++ b/src/corelib/io/qdir.cpp
@@ -50,6 +50,7 @@
 #include "qstring.h"
 #include "qregexp.h"
 #include "qvector.h"
+#include "qalgorithms.h"
 #ifdef QT_BUILD_CORE_LIB
 # include "qresource.h"
 #endif
@@ -190,32 +191,28 @@ QDirPrivate::~QDirPrivate()
 
 /* For sorting */
 struct QDirSortItem {
-    QString filename_cache;
-    QString suffix_cache;
+    mutable QString filename_cache;
+    mutable QString suffix_cache;
     QFileInfo item;
 };
-static int qt_cmp_si_sort_flags;
 
-#if defined(Q_C_CALLBACKS)
-extern "C" {
-#endif
 
-#ifdef Q_OS_WINCE
-static int __cdecl qt_cmp_si(const void *n1, const void *n2)
-#else
-static int qt_cmp_si(const void *n1, const void *n2)
-#endif
-{
-    if (!n1 || !n2)
-        return 0;
+class QDirSortItemComparator {
+    int qt_cmp_si_sort_flags;
+public:
+    QDirSortItemComparator(int flags) : qt_cmp_si_sort_flags(flags) {}
+    bool operator()(const QDirSortItem &, const QDirSortItem &);
+};
 
-    QDirSortItem* f1 = (QDirSortItem*)n1;
-    QDirSortItem* f2 = (QDirSortItem*)n2;
+bool QDirSortItemComparator::operator()(const QDirSortItem &n1, const QDirSortItem &n2)
+{
+    const QDirSortItem* f1 = &n1;
+    const QDirSortItem* f2 = &n2;
 
     if ((qt_cmp_si_sort_flags & QDir::DirsFirst) && (f1->item.isDir() != f2->item.isDir()))
-        return f1->item.isDir() ? -1 : 1;
+        return f1->item.isDir();
     if ((qt_cmp_si_sort_flags & QDir::DirsLast) && (f1->item.isDir() != f2->item.isDir()))
-        return f1->item.isDir() ? 1 : -1;
+        return !f1->item.isDir();
 
     int r = 0;
     int sortBy = (qt_cmp_si_sort_flags & QDir::SortByMask)
@@ -264,18 +261,11 @@ static int qt_cmp_si(const void *n1, const void *n2)
             : f1->filename_cache.compare(f2->filename_cache);
     }
 
-    if (r == 0) // Enforce an order - the order the items appear in the array
-        r = (char*)n1 - (char*)n2;
-
     if (qt_cmp_si_sort_flags & QDir::Reversed)
-        return -r;
-    return r;
+        return r > 0;
+    return r < 0;
 }
 
-#if defined(Q_C_CALLBACKS)
-}
-#endif
-
 inline void QDirPrivate::sortFileList(QDir::SortFlags sort, QStringList &l,
                                       QStringList *names, QFileInfoList *infos) const
 {
@@ -292,9 +282,8 @@ inline void QDirPrivate::sortFileList(QDir::SortFlags sort, QStringList &l,
                 path += QLatin1Char('/');
             si[i].item = QFileInfo(path + l.at(i));
         }
-        qt_cmp_si_sort_flags = sort;
         if ((sort & QDir::SortByMask) != QDir::Unsorted)
-            qsort(si, i, sizeof(si[0]), qt_cmp_si);
+            qStableSort(si, si+i, QDirSortItemComparator(sort));
         // put them back in the list(s)
         for (int j = 0; j<i; j++) {
             if(infos)