Epan+Qt: Invalidate cached column strings.

Add col_data_changed, which checks to see if we have updated column
info. Add col_append_frame_number, which adds a frame number and sets
col_data_changed. Call col_append_frame_number instead of
col_append_fstr from some dissectors.

Add PacketListRecord::invalidateAllRecords, which invalidates any cached
record data. Add PacketListModel::invalidateAllColumnStrings which calls
invalidateAllRecords and signals that our data has changed. Call
invalidateAllColumnStrings when we have new name resolution or column
information.

Bug: 11414
Bug: 11468
Change-Id: I2671594a722f4f9436fe1df84d43489a148e0cee
Reviewed-on: https://code.wireshark.org/review/26373
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>

18 files changed, 82 insertions, 24 deletions

diff --git a/debian/libwireshark0.symbols b/debian/libwireshark0.symbols
index 25d3530c73..eea1266fb4 100644
--- a/debian/libwireshark0.symbols
+++ b/debian/libwireshark0.symbols
@@ -98,6 +98,7 @@ libwireshark.so.0 libwireshark0 #MINVER#
  col_add_fstr@Base 1.9.1
  col_add_lstr@Base 1.12.0~rc1
  col_add_str@Base 1.9.1
+ col_append_frame_number@Base 2.5.1
  col_append_fstr@Base 1.9.1
  col_append_lstr@Base 1.99.0
  col_append_ports@Base 2.1.0
@@ -110,6 +111,7 @@ libwireshark.so.0 libwireshark0 #MINVER#
  col_clear@Base 1.9.1
  col_clear_fence@Base 1.12.0~rc1
  col_custom_prime_edt@Base 1.9.1
+ col_data_changed@Base 2.5.1
  col_fill_in@Base 1.9.1
  col_fill_in_error@Base 1.9.1
  col_fill_in_frame_data@Base 1.9.1
@@ -1518,6 +1520,7 @@ libwireshark.so.0 libwireshark0 #MINVER#
  tfs_accepted_not_accepted@Base 1.9.1
  tfs_ack_nack@Base 1.9.1
  tfs_acknowledged_not_acknowledged@Base 1.99.2
+ tfs_activated_deactivated@Base 2.5.1
  tfs_active_inactive@Base 1.9.1
  tfs_allocated_by_receiver_sender@Base 1.99.1
  tfs_allow_block@Base 1.9.1
@@ -1547,10 +1550,12 @@ libwireshark.so.0 libwireshark0 #MINVER#
  tfs_full_half@Base 1.99.1
  tfs_group_unique_name@Base 1.9.1
  tfs_high_low@Base 1.9.1
+ tfs_high_normal@Base 2.5.1
  tfs_implemented_not_implemented@Base 1.9.1
  tfs_included_not_included@Base 1.99.1
  tfs_inuse_not_inuse@Base 1.9.1
  tfs_invalid_valid@Base 1.99.0
+ tfs_low_normal@Base 2.5.1
  tfs_more_nomore@Base 1.9.1
  tfs_no_yes@Base 1.9.1
  tfs_not_allowed_allowed@Base 1.9.1
diff --git a/epan/column-utils.c b/epan/column-utils.c
index 42a9d8541f..9976804819 100644
--- a/epan/column-utils.c
+++ b/epan/column-utils.c
@@ -41,6 +41,9 @@
 /* Used for locale decimal point */
 static char *col_decimal_point;
 
+/* Used to indicate updated column information, e.g. a new request/response. */
+static gboolean col_data_changed_;
+
 /* Allocate all the data structures for constructing column data, given
    the number of columns. */
 void
@@ -456,6 +459,15 @@ col_append_ports(column_info *cinfo, const gint col, port_type typ, guint16 src,
   col_append_lstr(cinfo, col, buf_src, " " UTF8_RIGHTWARDS_ARROW " ", buf_dst, COL_ADD_LSTR_TERMINATOR);
 }
 
+void
+col_append_frame_number(packet_info *pinfo, const gint col, const gchar *fmt_str, guint frame_num)
+{
+  col_append_fstr(pinfo->cinfo, col, fmt_str, frame_num);
+  if (!pinfo->fd->flags.visited) {
+    col_data_changed_ = TRUE;
+  }
+}
+
 static void
 col_do_append_fstr(column_info *cinfo, const int el, const char *separator, const char *format, va_list ap)
 {
@@ -2282,6 +2294,10 @@ col_fill_in_error(column_info *cinfo, frame_data *fdata, const gboolean fill_col
   }
 }
 
+gboolean col_data_changed(void) {
+  return col_data_changed_;
+  col_data_changed_ = FALSE;
+}
 /*
  * Editor modelines
  *
diff --git a/epan/column-utils.h b/epan/column-utils.h
index 28755aaf0d..3463e00801 100644
--- a/epan/column-utils.h
+++ b/epan/column-utils.h
@@ -132,6 +132,12 @@ WS_DLL_PUBLIC void	col_fill_in(packet_info *pinfo, const gboolean fill_col_exprs
  */
 WS_DLL_PUBLIC void	col_fill_in_error(column_info *cinfo, frame_data *fdata, const gboolean fill_col_exprs, const gboolean fill_fd_colums);
 
+/** Check to see if our column data has changed, e.g. we have new request/response info.
+ *
+ * Internal, don't use this in dissectors!
+ */
+WS_DLL_PUBLIC gboolean	col_data_changed(void);
+
 /* Utility routines used by packet*.c */
 
 /** Are the columns writable?
@@ -271,6 +277,17 @@ WS_DLL_PUBLIC void col_append_str_uint(column_info *cinfo, const gint col, const
  */
 WS_DLL_PUBLIC void col_append_ports(column_info *cinfo, const gint col, port_type typ, guint16 src, guint16 dst);
 
+/** Append a frame number and signal that we have updated
+ * column information.
+ *
+ * @param pinfo the current packet info
+ * @param col the column to use, e.g. COL_INFO
+ * @param fmt_str Format string, e.g. "reassembled in %u".
+ * @param src the source port value to append
+ * @param dst the destination port value to append
+ */
+WS_DLL_PUBLIC void col_append_frame_number(packet_info *pinfo, const gint col, const gchar *fmt_str, guint frame_num);
+
 /* Append the given strings (terminated by COL_ADD_LSTR_TERMINATOR) to a column element,
  *
  * Same result as col_append_str() called for every string element.
diff --git a/epan/dissectors/packet-bthci_acl.c b/epan/dissectors/packet-bthci_acl.c
index 86db90e3f7..8b66e7d691 100644
--- a/epan/dissectors/packet-bthci_acl.c
+++ b/epan/dissectors/packet-bthci_acl.c
@@ -446,7 +446,7 @@ dissect_bthci_acl(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *dat
 
                 item = proto_tree_add_uint(bthci_acl_tree, hf_bthci_acl_reassembled_in, tvb, 0, 0, mfp->last_frame);
                 PROTO_ITEM_SET_GENERATED(item);
-                col_append_fstr(pinfo->cinfo, COL_INFO, " [Reassembled in #%u]", mfp->last_frame);
+                col_append_frame_number(pinfo, COL_INFO, " [Reassembled in #%u]", mfp->last_frame);
             }
         }
         if (pb_flag == 0x01) { /* continuation fragment */
@@ -466,11 +466,11 @@ dissect_bthci_acl(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *dat
 
                 item = proto_tree_add_uint(bthci_acl_tree, hf_bthci_acl_continuation_to, tvb, 0, 0, mfp->first_frame);
                 PROTO_ITEM_SET_GENERATED(item);
-                col_append_fstr(pinfo->cinfo, COL_INFO, " [Continuation to #%u]", mfp->first_frame);
+                col_append_frame_number(pinfo, COL_INFO, " [Continuation to #%u]", mfp->first_frame);
                 if (mfp->last_frame && mfp->last_frame != pinfo->num) {
                     item = proto_tree_add_uint(bthci_acl_tree, hf_bthci_acl_reassembled_in, tvb, 0, 0, mfp->last_frame);
                     PROTO_ITEM_SET_GENERATED(item);
-                    col_append_fstr(pinfo->cinfo, COL_INFO, " [Reassembled in #%u]", mfp->last_frame);
+                    col_append_frame_number(pinfo, COL_INFO, " [Reassembled in #%u]", mfp->last_frame);
                 }
             }
             if (mfp != NULL && mfp->last_frame == pinfo->num) {
diff --git a/epan/dissectors/packet-btl2cap.c b/epan/dissectors/packet-btl2cap.c
index 088244f0f7..d34a7e7124 100644
--- a/epan/dissectors/packet-btl2cap.c
+++ b/epan/dissectors/packet-btl2cap.c
@@ -2221,7 +2221,7 @@ dissect_i_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
             proto_item *item;
             item = proto_tree_add_uint(btl2cap_tree, hf_btl2cap_reassembled_in, tvb, 0, 0, mfp->last_frame);
             PROTO_ITEM_SET_GENERATED(item);
-            col_append_fstr(pinfo->cinfo, COL_INFO, "[Reassembled in #%u] ", mfp->last_frame);
+            col_append_frame_number(pinfo, COL_INFO, "[Reassembled in #%u] ", mfp->last_frame);
         }
     } else {
         if (length <= 4) {
diff --git a/epan/dissectors/packet-icmp.c b/epan/dissectors/packet-icmp.c
index 0bdb52d699..b8371a4b54 100644
--- a/epan/dissectors/packet-icmp.c
+++ b/epan/dissectors/packet-icmp.c
@@ -1056,7 +1056,7 @@ static icmp_transaction_t *transaction_start(packet_info * pinfo,
 					 icmp_trans->resp_frame);
 		PROTO_ITEM_SET_GENERATED(it);
 
-		col_append_fstr(pinfo->cinfo, COL_INFO, " (reply in %d)",
+		col_append_frame_number(pinfo, COL_INFO, " (reply in %u)",
 				icmp_trans->resp_frame);
 	}
 
@@ -1159,7 +1159,7 @@ static icmp_transaction_t *transaction_end(packet_info * pinfo,
 						"%.3f ms", resp_time);
 	PROTO_ITEM_SET_GENERATED(it);
 
-	col_append_fstr(pinfo->cinfo, COL_INFO, " (request in %d)",
+	col_append_frame_number(pinfo, COL_INFO, " (request in %d)",
 			icmp_trans->rqst_frame);
 
 	return icmp_trans;
diff --git a/epan/dissectors/packet-icmpv6.c b/epan/dissectors/packet-icmpv6.c
index 56633a6ee3..133df68e20 100644
--- a/epan/dissectors/packet-icmpv6.c
+++ b/epan/dissectors/packet-icmpv6.c
@@ -1467,7 +1467,7 @@ static icmp_transaction_t *transaction_start(packet_info *pinfo, proto_tree *tre
                 icmpv6_trans->resp_frame);
             PROTO_ITEM_SET_GENERATED(it);
         }
-        col_append_fstr(pinfo->cinfo, COL_INFO, " (reply in %d)", icmpv6_trans->resp_frame);
+        col_append_frame_number(pinfo, COL_INFO, " (reply in %d)", icmpv6_trans->resp_frame);
     }
 
     return icmpv6_trans;
@@ -1560,7 +1560,7 @@ static icmp_transaction_t *transaction_end(packet_info *pinfo, proto_tree *tree,
         PROTO_ITEM_SET_GENERATED(it);
     }
 
-    col_append_fstr(pinfo->cinfo, COL_INFO, " (request in %d)",
+    col_append_frame_number(pinfo, COL_INFO, " (request in %d)",
         icmpv6_trans->rqst_frame);
 
     return icmpv6_trans;
diff --git a/epan/dissectors/packet-ip.c b/epan/dissectors/packet-ip.c
index 479ac64488..d2a54b7c14 100644
--- a/epan/dissectors/packet-ip.c
+++ b/epan/dissectors/packet-ip.c
@@ -2266,7 +2266,7 @@ dissect_ip_v4(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void*
                  ipprotostr(iph->ip_proto), iph->ip_proto,
                  (iph->ip_off & IP_OFFSET) * 8, iph->ip_id);
     if ( ipfd_head && ipfd_head->reassembled_in != pinfo->num ) {
-      col_append_fstr(pinfo->cinfo, COL_INFO, " [Reassembled in #%u]",
+      col_append_frame_number(pinfo, COL_INFO, " [Reassembled in #%u]",
                       ipfd_head->reassembled_in);
     }
 
diff --git a/epan/dissectors/packet-iso15765.c b/epan/dissectors/packet-iso15765.c
index 13c3681c43..9ebbbf1cf9 100644
--- a/epan/dissectors/packet-iso15765.c
+++ b/epan/dissectors/packet-iso15765.c
@@ -325,7 +325,7 @@ dissect_iso15765(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data
                                                    &iso15765_frag_items, NULL, iso15765_tree);
 
                 if (frag_msg && frag_msg->reassembled_in != pinfo->num) {
-                    col_append_fstr(pinfo->cinfo, COL_INFO, " [Reassembled in #%u]",
+                    col_append_frame_number(pinfo, COL_INFO, " [Reassembled in #%u]",
                                     frag_msg->reassembled_in);
                 }
 
diff --git a/epan/dissectors/packet-ltp.c b/epan/dissectors/packet-ltp.c
index 79a1a144e9..c7da441276 100644
--- a/epan/dissectors/packet-ltp.c
+++ b/epan/dissectors/packet-ltp.c
@@ -412,7 +412,7 @@ dissect_data_segment(proto_tree *ltp_tree, tvbuff_t *tvb,packet_info *pinfo,int
 	{
 		if(frag_msg && more_frags)
 		{
-			col_append_fstr(pinfo->cinfo, COL_INFO, "[Reassembled in %d] ",frag_msg->reassembled_in);
+			col_append_frame_number(pinfo, COL_INFO, "[Reassembled in %d] ",frag_msg->reassembled_in);
 		}
 		else
 		{
diff --git a/epan/dissectors/packet-reload.c b/epan/dissectors/packet-reload.c
index 3f36c9ecdf..30641a23c3 100644
--- a/epan/dissectors/packet-reload.c
+++ b/epan/dissectors/packet-reload.c
@@ -4115,7 +4115,7 @@ dissect_reload_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void
       col_add_fstr(pinfo->cinfo, COL_INFO, "Fragmented RELOAD protocol (trans id=%x%x off=%u",
                    transaction_id[0],transaction_id[1], fragment);
       if (reload_fd_head && reload_fd_head->reassembled_in != pinfo->num) {
-        col_append_fstr(pinfo->cinfo, COL_INFO, " [Reassembled in #%u]",
+        col_append_frame_number(pinfo, COL_INFO, " [Reassembled in #%u]",
                         reload_fd_head->reassembled_in);
       }
       save_fragmented = pinfo->fragmented;
diff --git a/ui/qt/models/packet_list_model.cpp b/ui/qt/models/packet_list_model.cpp
index 4c5d4dc6e5..df94216544 100644
--- a/ui/qt/models/packet_list_model.cpp
+++ b/ui/qt/models/packet_list_model.cpp
@@ -136,6 +136,13 @@ void PacketListModel::clear() {
     idle_dissection_row_ = 0;
 }
 
+void PacketListModel::invalidateAllColumnStrings()
+{
+    PacketListRecord::invalidateAllRecords();
+    dataChanged(index(0, 0), index(rowCount() - 1, columnCount() - 1));
+    headerDataChanged(Qt::Horizontal, 0, columnCount() - 1);
+}
+
 void PacketListModel::resetColumns()
 {
     if (cap_file_) {
diff --git a/ui/qt/models/packet_list_model.h b/ui/qt/models/packet_list_model.h
index 9cd64df433..1b9635e274 100644
--- a/ui/qt/models/packet_list_model.h
+++ b/ui/qt/models/packet_list_model.h
@@ -51,6 +51,13 @@ public:
     frame_data *getRowFdata(int row);
     void ensureRowColorized(int row);
     int visibleIndexOf(frame_data *fdata) const;
+    /**
+     * @brief Invalidate any cached column strings.
+     */
+    void invalidateAllColumnStrings();
+    /**
+     * @brief Rebuild columns from settings.
+     */
     void resetColumns();
     void resetColorized();
     void toggleFrameMark(const QModelIndex &fm_index);
diff --git a/ui/qt/models/packet_list_record.cpp b/ui/qt/models/packet_list_record.cpp
index 8c1228b188..5c159fb2b6 100644
--- a/ui/qt/models/packet_list_record.cpp
+++ b/ui/qt/models/packet_list_record.cpp
@@ -72,7 +72,7 @@ const QByteArray PacketListRecord::columnString(capture_file *cap_file, int colu
 
 void PacketListRecord::resetColumns(column_info *cinfo)
 {
-    col_data_ver_++;
+    invalidateAllRecords();
 
     if (!cinfo) {
         return;
diff --git a/ui/qt/models/packet_list_record.h b/ui/qt/models/packet_list_record.h
index a5a623d8ab..0bfc1a7273 100644
--- a/ui/qt/models/packet_list_record.h
+++ b/ui/qt/models/packet_list_record.h
@@ -45,6 +45,7 @@ public:
     struct conversation *conversation() { return conv_; }
 
     int columnTextSize(const char *str);
+    static void invalidateAllRecords() { col_data_ver_++; }
     static void resetColumns(column_info *cinfo);
     void resetColorized();
     inline int lineCount() { return lines_; }
diff --git a/ui/qt/packet_list.cpp b/ui/qt/packet_list.cpp
index 5934523c5b..40c02703e0 100644
--- a/ui/qt/packet_list.cpp
+++ b/ui/qt/packet_list.cpp
@@ -308,6 +308,7 @@ PacketList::PacketList(QWidget *parent) :
     connect(packet_list_model_, SIGNAL(goToPacket(int)), this, SLOT(goToPacket(int)));
     connect(packet_list_model_, SIGNAL(itemHeightChanged(const QModelIndex&)), this, SLOT(updateRowHeights(const QModelIndex&)));
     connect(wsApp, SIGNAL(addressResolutionChanged()), this, SLOT(redrawVisiblePacketsDontSelectCurrent()));
+    connect(wsApp, SIGNAL(columnDataChanged()), this, SLOT(redrawVisiblePacketsDontSelectCurrent()));
 
     header()->setContextMenuPolicy(Qt::CustomContextMenu);
     connect(header(), SIGNAL(customContextMenuRequested(QPoint)),
@@ -699,16 +700,14 @@ void PacketList::drawCurrentPacket()
 // the UI to scroll to that packet).
 // Called from many places.
 void PacketList::redrawVisiblePackets() {
-    update();
-    header()->update();
+    redrawVisiblePacketsDontSelectCurrent();
     drawCurrentPacket();
 }
 
 // Redraw the packet list and detail.
 // Does not scroll back to the selected packet.
 void PacketList::redrawVisiblePacketsDontSelectCurrent() {
-    update();
-    header()->update();
+    packet_list_model_->invalidateAllColumnStrings();
 }
 
 void PacketList::resetColumns()
@@ -874,6 +873,9 @@ void PacketList::captureFileReadFinished()
 {
     packet_list_model_->flushVisibleRows();
     packet_list_model_->dissectIdle(true);
+    // Invalidating the column strings picks up and request/response
+    // tracking changes. We might just want to call it from flushVisibleRows.
+    packet_list_model_->invalidateAllColumnStrings();
 }
 
 void PacketList::freeze()
diff --git a/ui/qt/wireshark_application.cpp b/ui/qt/wireshark_application.cpp
index 24a5232c85..87399a37db 100644
--- a/ui/qt/wireshark_application.cpp
+++ b/ui/qt/wireshark_application.cpp
@@ -21,6 +21,7 @@
 #include "wsutil/filesystem.h"
 
 #include "epan/addr_resolv.h"
+#include "epan/column-utils.h"
 #include "epan/disabled_protos.h"
 #include "epan/ftypes/ftypes.h"
 #include "epan/prefs.h"
@@ -258,11 +259,12 @@ void WiresharkApplication::refreshRecentCaptures() {
     }
 }
 
-void WiresharkApplication::refreshAddressResolution()
+void WiresharkApplication::refreshPacketData()
 {
-    // Anything new show up?
     if (host_name_lookup_process()) {
         emit addressResolutionChanged();
+    } else if (col_data_changed()) {
+        emit columnDataChanged();
     }
 }
 
@@ -846,9 +848,9 @@ WiresharkApplication::WiresharkApplication(int &argc,  char **argv) :
     connect(&recent_timer_, SIGNAL(timeout()), this, SLOT(refreshRecentCaptures()));
     recent_timer_.start(2000);
 
-    addr_resolv_timer_.setParent(this);
-    connect(&addr_resolv_timer_, SIGNAL(timeout()), this, SLOT(refreshAddressResolution()));
-    addr_resolv_timer_.start(1000);
+    packet_data_timer_.setParent(this);
+    connect(&packet_data_timer_, SIGNAL(timeout()), this, SLOT(refreshPacketData()));
+    packet_data_timer_.start(1000);
 
     tap_update_timer_.setParent(this);
     tap_update_timer_.setInterval(TAP_UPDATE_DEFAULT_INTERVAL);
diff --git a/ui/qt/wireshark_application.h b/ui/qt/wireshark_application.h
index fa359b441e..24d662b0ae 100644
--- a/ui/qt/wireshark_application.h
+++ b/ui/qt/wireshark_application.h
@@ -137,7 +137,7 @@ private:
     QFont mono_font_;
     QFont zoomed_font_;
     QTimer recent_timer_;
-    QTimer addr_resolv_timer_;
+    QTimer packet_data_timer_;
     QTimer tap_update_timer_;
     QList<QString> pending_open_files_;
     QSocketNotifier *if_notifier_;
@@ -179,6 +179,7 @@ signals:
     void packetDissectionChanged();
     void preferencesChanged();
     void addressResolutionChanged();
+    void columnDataChanged();
     void checkDisplayFilter();
     void fieldsChanged();
     void reloadLuaPlugins();
@@ -209,7 +210,7 @@ private slots:
     void cleanup();
     void ifChangeEventsAvailable();
     void itemStatusFinished(const QString filename = "", qint64 size = 0, bool accessible = false);
-    void refreshAddressResolution();
+    void refreshPacketData();
 };
 
 extern WiresharkApplication *wsApp;