Tap parameter and stats dialog fixups.

If we run into an error when trying to register a tap listener, return
instead of tapping packets. This should fix some (but likely not all)
double frees found by Stig. For now close each statistics dialog if we
find an error. Note that we might want to keep them open instead.

Add checks and cleanups to some of the stats table free routines.

Call fillTree once in TapParameterDialog's constructor instead of each
time it's shown. Make fillTree a slot which lets us use a delay timer so
that the dialog is visible when we retap packets.

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

13 files changed, 51 insertions, 22 deletions

diff --git a/epan/rtd_table.c b/epan/rtd_table.c
index 1fe33d2b3a..40fe46b0f8 100644
--- a/epan/rtd_table.c
+++ b/epan/rtd_table.c
@@ -109,6 +109,8 @@ void free_rtd_table(rtd_stat_table* table, rtd_gui_free_cb gui_callback, void *c
         g_free(table->time_stats[i].rtd);
     }
     g_free(table->time_stats);
+    table->time_stats = NULL;
+    table->num_rtds = 0;
 
     /* Give GUI the first crack at it before we clean up */
     if (gui_callback)
diff --git a/epan/stats_tree.c b/epan/stats_tree.c
index aaad74e344..682dfbeb90 100644
--- a/epan/stats_tree.c
+++ b/epan/stats_tree.c
@@ -137,6 +137,8 @@ stats_tree_free(stats_tree *st)
     stat_node *child;
     stat_node *next;
 
+    if (!st) return;
+
     g_free(st->filter);
     g_hash_table_destroy(st->names);
     g_ptr_array_free(st->parents,TRUE);
diff --git a/ui/qt/response_time_delay_dialog.cpp b/ui/qt/response_time_delay_dialog.cpp
index 2e2de37c7a..193dbbaa80 100644
--- a/ui/qt/response_time_delay_dialog.cpp
+++ b/ui/qt/response_time_delay_dialog.cpp
@@ -256,7 +256,8 @@ void ResponseTimeDelayDialog::fillTree()
                              error_string->str);
         g_string_free(error_string, TRUE);
         free_rtd_table(&rtd_data.stat_table, NULL, NULL);
-        reject();
+        reject(); // XXX Stay open instead?
+        return;
     }
 
     statsTreeWidget()->setSortingEnabled(false);
diff --git a/ui/qt/response_time_delay_dialog.h b/ui/qt/response_time_delay_dialog.h
index 7d2fe13cab..0e5c6e56be 100644
--- a/ui/qt/response_time_delay_dialog.h
+++ b/ui/qt/response_time_delay_dialog.h
@@ -49,8 +49,10 @@ private:
     static void tapReset(void *rtdd_ptr);
     static void tapDraw(void *rtdd_ptr);
 
-    virtual void fillTree();
     virtual QList<QVariant> treeItemData(QTreeWidgetItem *ti) const;
+
+private slots:
+    virtual void fillTree();
 };
 
 /** Register function to register dissectors that support RTD.
diff --git a/ui/qt/rpc_service_response_time_dialog.h b/ui/qt/rpc_service_response_time_dialog.h
index 0814bd68c9..4998c90de4 100644
--- a/ui/qt/rpc_service_response_time_dialog.h
+++ b/ui/qt/rpc_service_response_time_dialog.h
@@ -59,7 +59,7 @@ public slots:
     void dceRpcProgramChanged(const QString &program_name);
     void oncRpcProgramChanged(const QString &program_name);
 
-protected:
+protected slots:
     virtual void fillTree();
 
 private:
diff --git a/ui/qt/service_response_time_dialog.cpp b/ui/qt/service_response_time_dialog.cpp
index b7e1d14b23..5219d3ceaf 100644
--- a/ui/qt/service_response_time_dialog.cpp
+++ b/ui/qt/service_response_time_dialog.cpp
@@ -278,7 +278,9 @@ void ServiceResponseTimeDialog::fillTree()
                              error_string->str);
         g_string_free(error_string, TRUE);
         g_array_free(srt_data.srt_array, TRUE);
-        reject();
+        srt_data.srt_array = NULL;
+        reject(); // XXX Stay open instead?
+        return;
     }
 
     statsTreeWidget()->setSortingEnabled(false);
diff --git a/ui/qt/service_response_time_dialog.h b/ui/qt/service_response_time_dialog.h
index 09bd7dbabc..b30a3e5e97 100644
--- a/ui/qt/service_response_time_dialog.h
+++ b/ui/qt/service_response_time_dialog.h
@@ -49,6 +49,8 @@ protected:
      */
     // gtk:service_response_table.h:init_srt_table
     void addSrtTable(const struct _srt_stat_table *srt_table);
+
+protected slots:
     virtual void fillTree();
 
 private:
diff --git a/ui/qt/simple_statistics_dialog.cpp b/ui/qt/simple_statistics_dialog.cpp
index 265dd085d9..525153580d 100644
--- a/ui/qt/simple_statistics_dialog.cpp
+++ b/ui/qt/simple_statistics_dialog.cpp
@@ -264,7 +264,8 @@ void SimpleStatisticsDialog::fillTree()
                              error_string->str);
         g_string_free(error_string, TRUE);
         free_stat_tables(stu_, NULL, NULL);
-        reject();
+        reject(); // XXX Stay open instead?
+        return;
     }
 
     cap_file_.retapPackets();
diff --git a/ui/qt/simple_statistics_dialog.h b/ui/qt/simple_statistics_dialog.h
index 918104e10d..5513112d9e 100644
--- a/ui/qt/simple_statistics_dialog.h
+++ b/ui/qt/simple_statistics_dialog.h
@@ -49,6 +49,7 @@ private:
     static void tapReset(void *sd_ptr);
     static void tapDraw(void *sd_ptr);
 
+private slots:
     virtual void fillTree();
 
 };
diff --git a/ui/qt/stats_tree_dialog.cpp b/ui/qt/stats_tree_dialog.cpp
index 83fe65245e..f4f804f673 100644
--- a/ui/qt/stats_tree_dialog.cpp
+++ b/ui/qt/stats_tree_dialog.cpp
@@ -150,7 +150,8 @@ void StatsTreeDialog::fillTree()
         QMessageBox::critical(this, tr("%1 failed to attach to tap").arg(display_name),
                              error_string->str);
         g_string_free(error_string, TRUE);
-        reject();
+        reject(); // XXX Stay open instead?
+        return;
     }
 
     cf_retap_packets(cap_file_.capFile());
@@ -182,7 +183,7 @@ void StatsTreeDialog::drawTreeItems(void *st_ptr)
     while (*iter) {
         stat_node *node = (*iter)->data(item_col_, Qt::UserRole).value<stat_node *>();
         if (node) {
-            gchar    **valstrs = stats_tree_get_values_from_node(node);
+            gchar **valstrs = stats_tree_get_values_from_node(node);
             for (int count = 0; count<st->num_columns; count++) {
                 (*iter)->setText(count,valstrs[count]);
                 g_free(valstrs[count]);
diff --git a/ui/qt/stats_tree_dialog.h b/ui/qt/stats_tree_dialog.h
index 0e810eb23c..9625b9b147 100644
--- a/ui/qt/stats_tree_dialog.h
+++ b/ui/qt/stats_tree_dialog.h
@@ -48,10 +48,12 @@ private:
     stats_tree *st_;
     stats_tree_cfg *st_cfg_;
 
-    virtual void fillTree();
     static void resetTap(void *st_ptr);
     static void drawTreeItems(void *st_ptr);
     virtual QByteArray getTreeAsString(st_format_type format);
+
+private slots:
+    virtual void fillTree();
 };
 
 #endif // STATS_TREE_DIALOG_H
diff --git a/ui/qt/tap_parameter_dialog.cpp b/ui/qt/tap_parameter_dialog.cpp
index 83837105d3..229ddc01d7 100644
--- a/ui/qt/tap_parameter_dialog.cpp
+++ b/ui/qt/tap_parameter_dialog.cpp
@@ -96,9 +96,20 @@ TapParameterDialog::TapParameterDialog(QWidget &parent, CaptureFile &cf, int hel
     button = ui->buttonBox->addButton(tr("Save as" UTF8_HORIZONTAL_ELLIPSIS), QDialogButtonBox::ActionRole);
     connect(button, SIGNAL(clicked()), this, SLOT(on_actionSaveAs_triggered()));
 
+    connect(ui->displayFilterLineEdit, SIGNAL(textChanged(QString)),
+            this, SLOT(updateWidgets()));
+
     if (help_topic_ < 1) {
         ui->buttonBox->button(QDialogButtonBox::Help)->hide();
     }
+
+    if (!ui->displayFilterLineEdit->text().isEmpty()) {
+        QString filter = ui->displayFilterLineEdit->text();
+        emit updateFilter(filter, true);
+    }
+    if (retap_on_show_) {
+        QTimer::singleShot(0, this, SLOT(fillTree()));
+    }
 }
 
 TapParameterDialog::~TapParameterDialog()
@@ -418,15 +429,6 @@ void TapParameterDialog::drawTreeItems()
     }
 }
 
-void TapParameterDialog::showEvent(QShowEvent *)
-{
-    if (!ui->displayFilterLineEdit->text().isEmpty()) {
-        QString filter = ui->displayFilterLineEdit->text();
-        emit updateFilter(filter, true);
-    }
-    if (retap_on_show_) fillTree();
-}
-
 void TapParameterDialog::contextMenuEvent(QContextMenuEvent *event)
 {
     bool enable = filterExpression().length() > 0 ? true : false;
@@ -487,14 +489,25 @@ void TapParameterDialog::addFilterActions()
 
 void TapParameterDialog::updateWidgets()
 {
+    bool edit_enable = true;
+    bool apply_enable = true;
+
     if (file_closed_) {
-        ui->displayFilterLineEdit->setEnabled(false);
-        ui->applyFilterButton->setEnabled(false);
+        edit_enable = false;
+        apply_enable = false;
+    } else if (!ui->displayFilterLineEdit->checkFilter()) {
+        // XXX Tell the user why the filter is invalid.
+        apply_enable = false;
     }
+    ui->displayFilterLineEdit->setEnabled(edit_enable);
+    ui->applyFilterButton->setEnabled(apply_enable);
 }
 
 void TapParameterDialog::on_applyFilterButton_clicked()
 {
+    if (!ui->displayFilterLineEdit->checkFilter())
+        return;
+
     QString filter = ui->displayFilterLineEdit->text();
     emit updateFilter(filter, true);
     fillTree();
diff --git a/ui/qt/tap_parameter_dialog.h b/ui/qt/tap_parameter_dialog.h
index 281274b298..5d9354bf5b 100644
--- a/ui/qt/tap_parameter_dialog.h
+++ b/ui/qt/tap_parameter_dialog.h
@@ -76,7 +76,6 @@ signals:
 public slots:
 
 protected:
-    void showEvent(QShowEvent *);
     void contextMenuEvent(QContextMenuEvent *event);
     void addFilterActions();
     QString displayFilter();
@@ -97,14 +96,15 @@ private:
     static const QString action_name_;
     bool retap_on_show_;
 
-    // Called by the constructor. The subclass should tap packets here.
-    virtual void fillTree() = 0;
     virtual const QString filterExpression() { return QString(); }
     QString itemDataToPlain(QVariant var, int width = 0);
     virtual QList<QVariant> treeItemData(QTreeWidgetItem *) const;
     virtual QByteArray getTreeAsString(st_format_type format);
 
 private slots:
+    // Called by the constructor. The subclass should tap packets here.
+    virtual void fillTree() = 0;
+
     void on_applyFilterButton_clicked();
     void on_actionCopyToClipboard_triggered();
     void on_actionSaveAs_triggered();