Qt: fix sorting of custom columns with multiple fields

Fields like "dns.time || http.time || smb.time" were sorted by column
number before. Recognize when all fields are numeric values and then try
to sort by number and otherwise fallback to a value comparison.

In theory sorting should now also be a bit faster for custom columns
because the columnn type is looked up once.

Change-Id: Id40d7cce8080d05823d74459fc493ec6ebf80956
Reported-by: Laura Chappell
Reviewed-on: https://code.wireshark.org/review/22317
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
(cherry picked from commit 16f70b9bb1faf48d985e21fcfa92d1934c2b50f5)
Reviewed-on: https://code.wireshark.org/review/22326
Reviewed-by: Stig Bjørlykke <stig@bjorlykke.org>

2 files changed, 37 insertions, 14 deletions

diff --git a/ui/qt/packet_list_model.cpp b/ui/qt/packet_list_model.cpp
index a46b6f568d..e1d60db9d2 100644
--- a/ui/qt/packet_list_model.cpp
+++ b/ui/qt/packet_list_model.cpp
@@ -295,6 +295,7 @@ void PacketListModel::setMaximiumRowHeight(int height)
 // to do in the future.
 
 int PacketListModel::sort_column_;
+int PacketListModel::sort_column_is_numeric_;
 int PacketListModel::text_sort_column_;
 Qt::SortOrder PacketListModel::sort_order_;
 capture_file *PacketListModel::sort_cap_file_;
@@ -343,6 +344,7 @@ void PacketListModel::sort(int column, Qt::SortOrder order)
     }
 
     busy_timer_.restart();
+    sort_column_is_numeric_ = isNumericColumn(sort_column_);
     std::sort(physical_rows_.begin(), physical_rows_.end(), recordLessThan);
 
     beginResetModel();
@@ -370,6 +372,38 @@ void PacketListModel::sort(int column, Qt::SortOrder order)
     }
 }
 
+bool PacketListModel::isNumericColumn(int column)
+{
+    if (column < 0 || sort_cap_file_->cinfo.columns[column].col_fmt != COL_CUSTOM) {
+        return false;
+    }
+
+    gchar **fields = g_regex_split_simple(COL_CUSTOM_PRIME_REGEX,
+            sort_cap_file_->cinfo.columns[column].col_custom_fields,
+            G_REGEX_ANCHORED, G_REGEX_MATCH_ANCHORED);
+
+    for (guint i = 0; i < g_strv_length(fields); i++) {
+        if (!*fields[i]) {
+            continue;
+        }
+
+        header_field_info *hfi = proto_registrar_get_byname(fields[i]);
+        if (!hfi || hfi->strings != NULL ||
+              !(((IS_FT_INT(hfi->type) || IS_FT_UINT(hfi->type)) &&
+                 ((hfi->display == BASE_DEC) || (hfi->display == BASE_DEC_HEX) ||
+                  (hfi->display == BASE_OCT))) ||
+                (hfi->type == FT_DOUBLE) || (hfi->type == FT_FLOAT) ||
+                (hfi->type == FT_BOOLEAN) || (hfi->type == FT_FRAMENUM) ||
+                (hfi->type == FT_RELATIVE_TIME))) {
+            g_strfreev(fields);
+            return false;
+        }
+    }
+
+    g_strfreev(fields);
+    return true;
+}
+
 bool PacketListModel::recordLessThan(PacketListRecord *r1, PacketListRecord *r2)
 {
     int cmp_val = 0;
@@ -394,21 +428,8 @@ bool PacketListModel::recordLessThan(PacketListRecord *r1, PacketListRecord *r2)
         if (r1->columnString(sort_cap_file_, sort_column_).constData() == r2->columnString(sort_cap_file_, sort_column_).constData()) {
             cmp_val = 0;
         } else if (sort_cap_file_->cinfo.columns[sort_column_].col_fmt == COL_CUSTOM) {
-            header_field_info *hfi;
-
             // Column comes from custom data
-            hfi = proto_registrar_get_byname(sort_cap_file_->cinfo.columns[sort_column_].col_custom_fields);
-
-            if (hfi == NULL) {
-                cmp_val = frame_data_compare(sort_cap_file_->epan, r1->frameData(), r2->frameData(), COL_NUMBER);
-            } else if ((hfi->strings == NULL) &&
-                       (((IS_FT_INT(hfi->type) || IS_FT_UINT(hfi->type)) &&
-                         ((hfi->display == BASE_DEC) || (hfi->display == BASE_DEC_HEX) ||
-                          (hfi->display == BASE_OCT))) ||
-                        (hfi->type == FT_DOUBLE) || (hfi->type == FT_FLOAT) ||
-                        (hfi->type == FT_BOOLEAN) || (hfi->type == FT_FRAMENUM) ||
-                        (hfi->type == FT_RELATIVE_TIME)))
-            {
+            if (sort_column_is_numeric_) {
                 // Attempt to convert to numbers.
                 // XXX This is slow. Can we avoid doing this?
                 bool ok_r1, ok_r2;
diff --git a/ui/qt/packet_list_model.h b/ui/qt/packet_list_model.h
index 3af6c60042..b6236e883a 100644
--- a/ui/qt/packet_list_model.h
+++ b/ui/qt/packet_list_model.h
@@ -106,6 +106,7 @@ private:
     int max_line_count_;
 
     static int sort_column_;
+    static int sort_column_is_numeric_;
     static int text_sort_column_;
     static Qt::SortOrder sort_order_;
     static capture_file *sort_cap_file_;
@@ -114,6 +115,7 @@ private:
     QElapsedTimer *idle_dissection_timer_;
     int idle_dissection_row_;
 
+    bool isNumericColumn(int column);
 
 private slots:
     void emitItemHeightChanged(const QModelIndex &ih_index);