Make color filters part of dissection

Color filters can be seen as a function that takes a tree and display
filters as input and a (possibly empty) coloring rule as output.
This coloring rule is then added to the frame tree and used by the GUI
for coloring the packets list.

From an architectural POV, "coloring" is part of the UI component, but
since it influences the tree, it is something for the dissection
component.

Bug: 5703
Bug: 6099
Change-Id: I73d132ec1dca7262bcb1b55c8481ca564c6161d1
Reviewed-on: https://code.wireshark.org/review/12507
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>

5 files changed, 55 insertions, 32 deletions

diff --git a/epan/color_filters.c b/epan/color_filters.c
index 01f9e3d45c..7c6fe5bb3c 100644
--- a/epan/color_filters.c
+++ b/epan/color_filters.c
@@ -39,6 +39,7 @@
 #include "file.h"
 #include <epan/dfilter/dfilter.h>
 #include <epan/prefs.h>
+#include <epan/epan_dissect.h>
 
 
 #define RED_COMPONENT(x)   (guint16) (((((x) >> 16) & 0xff) * 65535 / 255))
@@ -130,7 +131,7 @@ color_filters_find_by_name_cb(gconstpointer arg1, gconstpointer arg2)
     const color_filter_t *colorf = (const color_filter_t *)arg1;
     const gchar          *name   = (const gchar *)arg2;
 
-    return (strstr(colorf->filter_name, name)==NULL) ? -1 : 0 ;
+    return strcmp(colorf->filter_name, name);
 }
 
 
@@ -162,7 +163,7 @@ color_filters_set_tmp(guint8 filt_nr, const gchar *filter, gboolean disabled, gc
         /* Only change the filter rule if this is the rule to change or if
          * a matching filter string has been found
          */
-        if(colorf && ( (i==filt_nr) || (strstr(filter,colorf->filter_text)!=NULL) ) ) {
+        if(colorf && ( (i==filt_nr) || (!strcmp(filter, colorf->filter_text)) ) ) {
             /* set filter string to "frame" if we are resetting the rules
              * or if we found a matching filter string which need to be cleared
              */
@@ -489,7 +490,7 @@ color_filters_colorize_packet(epan_dissect_t *edt)
     color_filter_t *colorf;
 
     /* If we have color filters, "search" for the matching one. */
-    if (color_filters_used()) {
+    if ((edt->tree != NULL) && (color_filters_used())) {
         curr = color_filter_list;
 
         while(curr != NULL) {
diff --git a/epan/dissectors/file-file.c b/epan/dissectors/file-file.c
index e7a469a18e..e61d4310bf 100644
--- a/epan/dissectors/file-file.c
+++ b/epan/dissectors/file-file.c
@@ -88,13 +88,16 @@ call_file_record_end_routine(gpointer routine, gpointer dummy _U_)
 
 /* XXX - "packet comment" is passed into dissector as data, but currently doesn't have a use */
 static int
-dissect_file_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* data _U_)
+dissect_file_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* data)
 {
 	proto_item  *volatile ti = NULL;
 	guint	     cap_len = 0, frame_len = 0;
+	proto_tree  *volatile fh_tree = NULL;
 	proto_tree  *volatile tree;
 	proto_item  *item;
 	const gchar *cap_plurality, *frame_plurality;
+	const color_filter_t *color_filter;
+	file_data_t *file_data = (file_data_t*)data;
 
 	tree=parent_tree;
 
@@ -105,7 +108,6 @@ dissect_file_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree,
 	if(!proto_field_is_referenced(tree, proto_file)) {
 		tree=NULL;
 	} else {
-		proto_tree *fh_tree;
 		gboolean old_visible;
 
 		/* Put in frame header information. */
@@ -171,16 +173,6 @@ dissect_file_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree,
 						    pinfo->fd->file_off, pinfo->fd->file_off);
 		}
 #endif
-
-		if(pinfo->fd->color_filter != NULL) {
-			const color_filter_t *color_filter = (const color_filter_t *)pinfo->fd->color_filter;
-			item = proto_tree_add_string(fh_tree, hf_file_color_filter_name, tvb,
-						     0, 0, color_filter->filter_name);
-			PROTO_ITEM_SET_GENERATED(item);
-			item = proto_tree_add_string(fh_tree, hf_file_color_filter_text, tvb,
-						     0, 0, color_filter->filter_text);
-			PROTO_ITEM_SET_GENERATED(item);
-		}
 	}
 
 	if (pinfo->fd->flags.ignored) {
@@ -306,6 +298,19 @@ dissect_file_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree,
 		ENDTRY;
 	}
 
+	/* XXX optimize this so it doesn't need to scan the second time */
+	color_filter = color_filters_colorize_packet(file_data->color_edt);
+
+	if (color_filter) {
+		pinfo->fd->color_filter = color_filter;
+		item = proto_tree_add_string(fh_tree, hf_file_color_filter_name, tvb,
+					     0, 0, color_filter->filter_name);
+		PROTO_ITEM_SET_GENERATED(item);
+		item = proto_tree_add_string(fh_tree, hf_file_color_filter_text, tvb,
+					     0, 0, color_filter->filter_text);
+		PROTO_ITEM_SET_GENERATED(item);
+	}
+
 	tap_queue_packet(file_tap, pinfo, NULL);
 
 
diff --git a/epan/dissectors/packet-frame.c b/epan/dissectors/packet-frame.c
index 2a7b498ac4..2cec2d50fa 100644
--- a/epan/dissectors/packet-frame.c
+++ b/epan/dissectors/packet-frame.c
@@ -178,9 +178,11 @@ dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void*
 	guint	     cap_len = 0, frame_len = 0;
 	proto_tree  *volatile tree;
 	proto_tree  *comments_tree;
+	proto_tree  *volatile fh_tree = NULL;
 	proto_item  *item;
 	const gchar *cap_plurality, *frame_plurality;
 	frame_data_t *fr_data = (frame_data_t*)data;
+	const color_filter_t *color_filter;
 
 	tree=parent_tree;
 
@@ -273,7 +275,6 @@ dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void*
 				expert_add_info(pinfo, NULL, &ei_arrive_time_out_of_range);
 		}
 	} else {
-		proto_tree *fh_tree;
 		gboolean old_visible;
 
 		/* Put in frame header information. */
@@ -457,16 +458,6 @@ dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void*
 						    "%" G_GINT64_MODIFIER "d (0x%" G_GINT64_MODIFIER "x)",
 						    pinfo->fd->file_off, pinfo->fd->file_off);
 		}
-
-		if (pinfo->fd->color_filter != NULL) {
-			const color_filter_t *color_filter = (const color_filter_t *)pinfo->fd->color_filter;
-			item = proto_tree_add_string(fh_tree, hf_frame_color_filter_name, tvb,
-						     0, 0, color_filter->filter_name);
-			PROTO_ITEM_SET_GENERATED(item);
-			item = proto_tree_add_string(fh_tree, hf_frame_color_filter_text, tvb,
-						     0, 0, color_filter->filter_text);
-			PROTO_ITEM_SET_GENERATED(item);
-		}
 	}
 
 	if (pinfo->fd->flags.ignored) {
@@ -625,6 +616,19 @@ dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void*
 		ENDTRY;
 	}
 
+	/* XXX optimize this so it doesn't need to scan the second time */
+	color_filter = color_filters_colorize_packet(fr_data->color_edt);
+
+	if (color_filter) {
+		pinfo->fd->color_filter = color_filter;
+		item = proto_tree_add_string(fh_tree, hf_frame_color_filter_name, tvb,
+					     0, 0, color_filter->filter_name);
+		PROTO_ITEM_SET_GENERATED(item);
+		item = proto_tree_add_string(fh_tree, hf_frame_color_filter_text, tvb,
+					     0, 0, color_filter->filter_text);
+		PROTO_ITEM_SET_GENERATED(item);
+	}
+
 	tap_queue_packet(frame_tap, pinfo, NULL);
 
 
diff --git a/epan/packet.c b/epan/packet.c
index 0e3c02b5f3..71c814191b 100644
--- a/epan/packet.c
+++ b/epan/packet.c
@@ -489,6 +489,7 @@ dissect_record(epan_dissect_t *edt, int file_type_subtype,
 	else
 		frame_dissector_data.pkt_comment = NULL;
 	frame_dissector_data.file_type_subtype = file_type_subtype;
+	frame_dissector_data.color_edt = edt; /* Used strictly for "coloring rules" */
 
 	TRY {
 		/* Add this tvbuffer into the data_src list */
@@ -518,6 +519,8 @@ void
 dissect_file(epan_dissect_t *edt, struct wtap_pkthdr *phdr,
 	       tvbuff_t *tvb, frame_data *fd, column_info *cinfo)
 {
+	file_data_t file_dissector_data;
+
 	if (cinfo != NULL)
 		col_init(cinfo, edt->session);
 	edt->pi.epan = edt->session;
@@ -546,15 +549,15 @@ dissect_file(epan_dissect_t *edt, struct wtap_pkthdr *phdr,
 
 
 	TRY {
-		const gchar *pkt_comment;
-
 		/* pkt comment use first user, later from phdr */
 		if (fd->flags.has_user_comment)
-			pkt_comment = epan_get_user_comment(edt->session, fd);
+			file_dissector_data.pkt_comment = epan_get_user_comment(edt->session, fd);
 		else if (fd->flags.has_phdr_comment)
-			pkt_comment = phdr->opt_comment;
+			file_dissector_data.pkt_comment = phdr->opt_comment;
 		else
-			pkt_comment = NULL;
+			file_dissector_data.pkt_comment = NULL;
+		file_dissector_data.color_edt = edt; /* Used strictly for "coloring rules" */
+
 
 		/* Add this tvbuffer into the data_src list */
 		add_new_data_source(&edt->pi, edt->tvb, "File");
@@ -563,7 +566,7 @@ dissect_file(epan_dissect_t *edt, struct wtap_pkthdr *phdr,
 		 * sub-dissector can throw, dissect_frame() itself may throw
 		 * a ReportedBoundsError in bizarre cases. Thus, we catch the exception
 		 * in this function. */
-		call_dissector_with_data(file_handle, edt->tvb, &edt->pi, edt->tree, (void*)pkt_comment);
+		call_dissector_with_data(file_handle, edt->tvb, &edt->pi, edt->tree, &file_dissector_data);
 
 	}
 	CATCH(BoundsError) {
diff --git a/epan/packet.h b/epan/packet.h
index c748caabd4..0bb96da3e2 100644
--- a/epan/packet.h
+++ b/epan/packet.h
@@ -662,8 +662,18 @@ typedef struct frame_data_s
 {
     int file_type_subtype;
     const gchar  *pkt_comment; /**< NULL if not available */
+    struct epan_dissect *color_edt; /** Used strictly for "coloring rules" */
+
 } frame_data_t;
 
+/* Structure passed to the file dissector */
+typedef struct file_data_s
+{
+    const gchar  *pkt_comment; /**< NULL if not available */
+    struct epan_dissect *color_edt; /** Used strictly for "coloring rules" */
+
+} file_data_t;
+
 /*
  * Dissectors should never modify the record data.
  */