Fix a "recent items" crash.

Use a QMutableListIterator instead of a foreach loop so that we can
safely remove items from a list.

While we're here, make sure that RecentFileStatus threads use a
Qt::QueuedConnection when emitting signals across threads and try to
isolate the filename string.

Change-Id: I3fbb65a1727133f4557026decab5084a3faec2d4
Reviewed-on: https://code.wireshark.org/review/2966
Reviewed-by: Gerald Combs <gerald@wireshark.org>

3 files changed, 19 insertions, 23 deletions

diff --git a/ui/qt/recent_file_status.h b/ui/qt/recent_file_status.h
index 70800b8f35..b3cef5c9dd 100644
--- a/ui/qt/recent_file_status.h
+++ b/ui/qt/recent_file_status.h
@@ -28,7 +28,7 @@ class RecentFileStatus : public QObject
 {
     Q_OBJECT
 public:
-    RecentFileStatus(const QString &filename, QObject *parent = 0) :
+    RecentFileStatus(const QString filename, QObject *parent = 0) :
         QObject(parent), filename_(filename), size_(0) {}
 
     QString getFilename() const { return (filename_); }
@@ -40,7 +40,7 @@ private:
     size_t size_;
 
 signals:
-    void statusFound(const QString &filename = *new QString(), qint64 size = 0, bool accessible = false);
+    void statusFound(const QString filename = QString(), qint64 size = 0, bool accessible = false);
     void finished();
 
 public slots:
diff --git a/ui/qt/wireshark_application.cpp b/ui/qt/wireshark_application.cpp
index b6cbeb62f8..eaa0ba24f7 100644
--- a/ui/qt/wireshark_application.cpp
+++ b/ui/qt/wireshark_application.cpp
@@ -55,6 +55,7 @@
 #include <QEvent>
 #include <QFileOpenEvent>
 #include <QFontMetrics>
+#include <QMutableListIterator>
 #include <QTimer>
 #include <QUrl>
 
@@ -70,7 +71,7 @@ WiresharkApplication *wsApp = NULL;
 // MUST be UTF-8
 static char *last_open_dir = NULL;
 static bool updated_last_open_dir = FALSE;
-static QList<recent_item_status *> recent_items;
+static QList<recent_item_status *> recent_items_;
 
 void
 topic_action(topic_action_e action)
@@ -108,13 +109,13 @@ add_menu_recent_capture_file(const gchar *cf_name) {
     normalized_cf_name = QDir::cleanPath(normalized_cf_name);
     normalized_cf_name = QDir::toNativeSeparators(normalized_cf_name);
 
-    recent_item_status *ri;
-
     /* Iterate through the recent items list, removing duplicate entries and every
      * item above count_max
      */
     unsigned int cnt = 1;
-    foreach (ri, wsApp->recentItems()) {
+    QMutableListIterator<recent_item_status *> rii(recent_items_);
+    while (rii.hasNext()) {
+        recent_item_status *ri = rii.next();
         /* if this element string is one of our special items (separator, ...) or
          * already in the list or
          * this element is above maximum count (too old), remove it
@@ -128,7 +129,7 @@ add_menu_recent_capture_file(const gchar *cf_name) {
             ri->filename.compare(normalized_cf_name) == 0 ||
 #endif
             cnt >= prefs.gui_recent_files_count_max) {
-            wsApp->recentItems().removeOne(ri);
+            rii.remove();
             delete(ri);
             cnt--;
         }
@@ -143,7 +144,7 @@ extern "C" void menu_recent_file_write_all(FILE *rf) {
     /* we have to iterate backwards through the children's list,
      * so we get the latest item last in the file.
      */
-    QListIterator<recent_item_status *> rii(recent_items);
+    QListIterator<recent_item_status *> rii(recent_items_);
     rii.toBack();
     while (rii.hasPrevious()) {
         QString cf_name;
@@ -163,7 +164,7 @@ void WiresharkApplication::refreshRecentFiles(void) {
     RecentFileStatus *rf_status;
     QThread *rf_thread;
 
-    foreach (ri, recent_items) {
+    foreach (ri, recent_items_) {
         if (ri->in_thread) {
             continue;
         }
@@ -175,7 +176,8 @@ void WiresharkApplication::refreshRecentFiles(void) {
 
         connect(rf_thread, SIGNAL(started()), rf_status, SLOT(start()));
 
-        connect(rf_status, SIGNAL(statusFound(QString, qint64, bool)), this, SLOT(itemStatusFinished(QString, qint64, bool)));
+        connect(rf_status, SIGNAL(statusFound(QString, qint64, bool)),
+                this, SLOT(itemStatusFinished(QString, qint64, bool)), Qt::QueuedConnection);
         connect(rf_status, SIGNAL(finished()), rf_thread, SLOT(quit()));
         connect(rf_status, SIGNAL(finished()), rf_status, SLOT(deleteLater()));
 
@@ -516,12 +518,8 @@ bool WiresharkApplication::event(QEvent *event)
 }
 
 void WiresharkApplication::clearRecentItems() {
-    recent_item_status *ri;
-
-    foreach (ri, recent_items) {
-        recent_items.removeOne(ri);
-        delete(ri);
-    }
+    qDeleteAll(recent_items_.begin(), recent_items_.end());
+    recent_items_.clear();
     emit updateRecentItemStatus(NULL, 0, false);
 }
 
@@ -534,18 +532,16 @@ void WiresharkApplication::cleanup()
     write_recent();
 }
 
-void WiresharkApplication::itemStatusFinished(const QString &filename, qint64 size, bool accessible) {
+void WiresharkApplication::itemStatusFinished(const QString filename, qint64 size, bool accessible) {
     recent_item_status *ri;
     RecentFileStatus *rf_status = qobject_cast<RecentFileStatus *>(QObject::sender());
 
-//    g_log(NULL, G_LOG_LEVEL_DEBUG, "rf isf %d", recent_items.count());
-    foreach (ri, recent_items) {
+    foreach (ri, recent_items_) {
         if (filename == ri->filename && (size != ri->size || accessible != ri->accessible)) {
             ri->size = size;
             ri->accessible = accessible;
             ri->in_thread = false;
 
-//            g_log(NULL, G_LOG_LEVEL_DEBUG, "rf update %s", filename.toUtf8().constData());
             emit updateRecentItemStatus(filename, size, accessible);
         }
     }
@@ -739,7 +735,7 @@ e_prefs * WiresharkApplication::readConfigurationFiles(char **gdp_path, char **d
 }
 
 QList<recent_item_status *> WiresharkApplication::recentItems() const {
-    return recent_items;
+    return recent_items_;
 }
 
 void WiresharkApplication::addRecentItem(const QString &filename, qint64 size, bool accessible) {
@@ -749,7 +745,7 @@ void WiresharkApplication::addRecentItem(const QString &filename, qint64 size, b
     ri->size = size;
     ri->accessible = accessible;
     ri->in_thread = false;
-    recent_items.prepend(ri);
+    recent_items_.prepend(ri);
 
     itemStatusFinished(filename, size, accessible);
 }
diff --git a/ui/qt/wireshark_application.h b/ui/qt/wireshark_application.h
index 0c78055982..120d552bce 100644
--- a/ui/qt/wireshark_application.h
+++ b/ui/qt/wireshark_application.h
@@ -132,7 +132,7 @@ public slots:
 
 private slots:
     void cleanup();
-    void itemStatusFinished(const QString &filename = "", qint64 size = 0, bool accessible = false);
+    void itemStatusFinished(const QString filename = "", qint64 size = 0, bool accessible = false);
     void refreshRecentFiles(void);
     void updateTaps();
 };