Sequence analysis (flow graph) optimizations for dissectors

1. Remove protocol member from seq_analysis_item_t.
It's not used by any GUI, so don't burden dissectors with populating it.

2. Allow any dissector to change colors display by flow graph

3. Provide helper functions that may be common if other dissectors
want to create sequence analysis.

Change-Id: I04fa3c9f3cf6879ab9a8d7d6f4896b4979d010d7
Reviewed-on: https://code.wireshark.org/review/23613
Reviewed-by: Michael Mann <mmann78@netscape.net>
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Jakub Zawadzki <darkjames-ws@darkjames.pl>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

4 files changed, 91 insertions, 82 deletions

diff --git a/epan/dissectors/packet-frame.c b/epan/dissectors/packet-frame.c
index 9b9a0dbaf6..83ace71d37 100644
--- a/epan/dissectors/packet-frame.c
+++ b/epan/dissectors/packet-frame.c
@@ -170,99 +170,32 @@ frame_seq_analysis_packet( void *ptr, packet_info *pinfo, epan_dissect_t *edt _U
 	seq_analysis_info_t *sainfo = (seq_analysis_info_t *) ptr;
 
 	if ((sainfo->all_packets) || (pinfo->fd->flags.passed_dfilter == 1)) {
-		gchar *protocol = NULL;
-		gchar *colinfo = NULL;
-		seq_analysis_item_t *sai = NULL;
-
-		if (sainfo->any_addr) {
-			if (pinfo->net_src.type!=AT_NONE && pinfo->net_dst.type!=AT_NONE) {
-				sai = g_new0(seq_analysis_item_t, 1);
-				copy_address(&(sai->src_addr),&(pinfo->net_src));
-				copy_address(&(sai->dst_addr),&(pinfo->net_dst));
-			}
-
-		} else {
-			if (pinfo->src.type!=AT_NONE && pinfo->dst.type!=AT_NONE) {
-				sai = g_new0(seq_analysis_item_t, 1);
-				copy_address(&(sai->src_addr),&(pinfo->src));
-				copy_address(&(sai->dst_addr),&(pinfo->dst));
-			}
-		}
 
+		seq_analysis_item_t *sai = sequence_analysis_create_sai_with_addresses(pinfo, sainfo);
 		if (!sai)
 			return FALSE;
 
 		sai->frame_number = pinfo->num;
 
-		if (pinfo->fd->color_filter) {
-			sai->bg_color = color_t_to_rgb(&pinfo->fd->color_filter->bg_color);
-			sai->fg_color = color_t_to_rgb(&pinfo->fd->color_filter->fg_color);
-			sai->has_color_filter = TRUE;
-		}
+		sequence_analysis_use_color_filter(pinfo, sai);
 
 		sai->port_src=pinfo->srcport;
 		sai->port_dst=pinfo->destport;
-		sai->protocol = g_strdup(port_type_to_str(pinfo->ptype));
-
-		if (pinfo->cinfo) {
-			col_item_t *col_item;
-			int i;
-
-			if (pinfo->cinfo->col_first[COL_INFO] >= 0) {
-				for (i = pinfo->cinfo->col_first[COL_INFO]; i <= pinfo->cinfo->col_last[COL_INFO]; i++) {
-					col_item = &pinfo->cinfo->columns[i];
-					if (col_item->fmt_matx[COL_INFO]) {
-						colinfo = g_strdup(col_item->col_data);
-						/* break; ? or g_free(colinfo); before g_strdup() */
-					}
-				}
-			}
-
-			if (pinfo->cinfo->col_first[COL_PROTOCOL] >= 0) {
-				for (i = pinfo->cinfo->col_first[COL_PROTOCOL]; i <= pinfo->cinfo->col_last[COL_PROTOCOL]; i++) {
-					col_item = &pinfo->cinfo->columns[i];
-					if (col_item->fmt_matx[COL_PROTOCOL]) {
-						protocol = g_strdup(col_item->col_data);
-						/* break; ? or g_free(protocol); before g_strdup() */
-					}
-				}
-			}
-		}
 
-		if (colinfo != NULL) {
-			sai->frame_label = g_strdup(colinfo);
-			if (protocol != NULL) {
-				sai->comment = g_strdup_printf("%s: %s", protocol, colinfo);
-			} else {
-				sai->comment = g_strdup(colinfo);
-			}
-		} else {
-			/* This will probably never happen...*/
-			if (protocol != NULL) {
-				sai->frame_label = g_strdup(protocol);
-				sai->comment = g_strdup(protocol);
-			}
-		}
+		sequence_analysis_use_col_info_as_label_comment(pinfo, sai);
 
 		if (pinfo->ptype == PT_NONE) {
 			icmp_info_t *p_icmp_info;
 
 			if ((p_icmp_info = (icmp_info_t *) p_get_proto_data(wmem_file_scope(), pinfo, proto_get_id_by_short_name("ICMP"), 0))) {
-				g_free(sai->protocol);
-				sai->protocol = g_strdup("ICMP");
 				sai->port_src = 0;
 				sai->port_dst = p_icmp_info->type * 256 + p_icmp_info->code;
 			} else if ((p_icmp_info = (icmp_info_t *) p_get_proto_data(wmem_file_scope(), pinfo, proto_get_id_by_short_name("ICMPv6"), 0))) {
-				g_free(sai->protocol);
-				sai->protocol = g_strdup("ICMPv6");
 				sai->port_src = 0;
 				sai->port_dst = p_icmp_info->type * 256 + p_icmp_info->code;
 			}
 		}
 
-		g_free(protocol);
-		g_free(colinfo);
-
 		sai->line_style = 1;
 		sai->conv_num = 0;
 		sai->display = TRUE;
diff --git a/epan/dissectors/packet-tcp.c b/epan/dissectors/packet-tcp.c
index 899326f6a6..4c6bded5ff 100644
--- a/epan/dissectors/packet-tcp.c
+++ b/epan/dissectors/packet-tcp.c
@@ -872,20 +872,15 @@ tcp_seq_analysis_packet( void *ptr, packet_info *pinfo, epan_dissect_t *edt _U_,
 
     if ((sainfo->all_packets)||(pinfo->fd->flags.passed_dfilter==1)){
         const char* flags;
-        seq_analysis_item_t *sai;
+        seq_analysis_item_t *sai = sequence_analysis_create_sai_with_addresses(pinfo, sainfo);
+
+        if (!sai)
+            return FALSE;
 
-        sai = g_new0(seq_analysis_item_t, 1);
         sai->frame_number = pinfo->num;
-        if (sainfo->any_addr) {
-            copy_address(&(sai->src_addr),&(pinfo->net_src));
-            copy_address(&(sai->dst_addr),&(pinfo->net_dst));
-        } else {
-            copy_address(&(sai->src_addr),&(pinfo->src));
-            copy_address(&(sai->dst_addr),&(pinfo->dst));
-        }
+
         sai->port_src=pinfo->srcport;
         sai->port_dst=pinfo->destport;
-        sai->protocol=g_strdup(port_type_to_str(pinfo->ptype));
 
         flags = tcp_flags_to_str(NULL, tcph);
 
diff --git a/epan/sequence_analysis.c b/epan/sequence_analysis.c
index 7d8f03064f..e829f48746 100644
--- a/epan/sequence_analysis.c
+++ b/epan/sequence_analysis.c
@@ -28,6 +28,8 @@
 
 #include "addr_resolv.h"
 #include "proto.h"
+#include "color_filters.h"
+#include "column-info.h"
 #include "tap.h"
 #include "wmem/wmem.h"
 
@@ -105,6 +107,63 @@ void sequence_analysis_table_iterate_tables(wmem_foreach_func func, gpointer use
     wmem_tree_foreach(registered_seq_analysis, func, user_data);
 }
 
+seq_analysis_item_t* sequence_analysis_create_sai_with_addresses(packet_info *pinfo, seq_analysis_info_t *sainfo)
+{
+    seq_analysis_item_t *sai = NULL;
+
+    if (sainfo->any_addr) {
+        if (pinfo->net_src.type!=AT_NONE && pinfo->net_dst.type!=AT_NONE) {
+            sai = g_new0(seq_analysis_item_t, 1);
+            copy_address(&(sai->src_addr),&(pinfo->net_src));
+            copy_address(&(sai->dst_addr),&(pinfo->net_dst));
+        }
+
+    } else {
+        if (pinfo->src.type!=AT_NONE && pinfo->dst.type!=AT_NONE) {
+            sai = g_new0(seq_analysis_item_t, 1);
+            copy_address(&(sai->src_addr),&(pinfo->src));
+            copy_address(&(sai->dst_addr),&(pinfo->dst));
+        }
+    }
+
+    return sai;
+}
+
+void sequence_analysis_use_color_filter(packet_info *pinfo, seq_analysis_item_t *sai)
+{
+    if (pinfo->fd->color_filter) {
+        sai->bg_color = color_t_to_rgb(&pinfo->fd->color_filter->bg_color);
+        sai->fg_color = color_t_to_rgb(&pinfo->fd->color_filter->fg_color);
+        sai->has_color_filter = TRUE;
+    }
+}
+
+void sequence_analysis_use_col_info_as_label_comment(packet_info *pinfo, seq_analysis_item_t *sai)
+{
+    const gchar *protocol = NULL;
+    const gchar *colinfo = NULL;
+
+    if (pinfo->cinfo) {
+        colinfo = col_get_text(pinfo->cinfo, COL_INFO);
+        protocol = col_get_text(pinfo->cinfo, COL_PROTOCOL);
+    }
+
+    if (colinfo != NULL) {
+        sai->frame_label = g_strdup(colinfo);
+        if (protocol != NULL) {
+            sai->comment = g_strdup_printf("%s: %s", protocol, colinfo);
+        } else {
+            sai->comment = g_strdup(colinfo);
+        }
+    } else {
+        /* This will probably never happen...*/
+        if (protocol != NULL) {
+            sai->frame_label = g_strdup(protocol);
+            sai->comment = g_strdup(protocol);
+        }
+    }
+}
+
 seq_analysis_info_t *
 sequence_analysis_info_new(void)
 {
@@ -135,7 +194,6 @@ static void sequence_analysis_item_free(gpointer data)
     g_free(seq_item->frame_label);
     g_free(seq_item->time_str);
     g_free(seq_item->comment);
-    g_free(seq_item->protocol);
     free_address(&seq_item->src_addr);
     free_address(&seq_item->dst_addr);
     g_free(data);
diff --git a/epan/sequence_analysis.h b/epan/sequence_analysis.h
index bcebdef3a5..247b3d5fbd 100644
--- a/epan/sequence_analysis.h
+++ b/epan/sequence_analysis.h
@@ -63,7 +63,6 @@ typedef struct _seq_analysis_item {
     guint src_node;                     /**< this is used by graph_analysis.c to identify the node */
     guint dst_node;                     /**< a node is an IP address that will be displayed in columns */
     guint16 line_style;                 /**< the arrow line width in pixels*/
-    gchar *protocol;                    /**< the label of the protocol defined in the IP packet */
 } seq_analysis_item_t;
 
 /** defines the graph analysis structure */
@@ -128,6 +127,30 @@ WS_DLL_PUBLIC tap_packet_cb sequence_analysis_get_packet_func(register_analysis_
  */
 WS_DLL_PUBLIC guint sequence_analysis_get_tap_flags(register_analysis_t* analysis);
 
+/** Helper function to create a sequence analysis item with address fields populated
+ * Allocate a seq_analysis_item_t to return and populate the src_addr and dst_addr
+ * members based on seq_analysis_info_t any_addr member
+ *
+ * @param pinfo packet info
+ * @param sainfo info determining address type
+ * @return sequence analysis tap flags
+ */
+WS_DLL_PUBLIC seq_analysis_item_t* sequence_analysis_create_sai_with_addresses(packet_info *pinfo, seq_analysis_info_t *sainfo);
+
+/** Helper function to set colors for analysis the same as Wireshark display
+ *
+ * @param pinfo packet info
+ * @param sai item to set color
+ */
+WS_DLL_PUBLIC void sequence_analysis_use_color_filter(packet_info *pinfo, seq_analysis_item_t *sai);
+
+/** Helper function to set frame label and comments to use protocol and info column data
+ *
+ * @param pinfo packet info
+ * @param sai item to set label and comments
+ */
+WS_DLL_PUBLIC void sequence_analysis_use_col_info_as_label_comment(packet_info *pinfo, seq_analysis_item_t *sai);
+
 /** Find a registered sequence analysis "protocol" by name
  *
  * @param name Registered sequence analysis to find