From Kovarththanan Rajaratnam via bug 3719:

This patch optimizes proto_tree_prime_hfid() + friends and
plugs a memleak in the process.

From me:
Removed unused hfindex in proto_tree_new_item()
Fixed ref_count entry in struct header_field_info.

svn path=/trunk/; revision=29137

5 files changed, 90 insertions, 51 deletions

diff --git a/epan/dissectors/packet-xml.c b/epan/dissectors/packet-xml.c
index 539a875f46..4a734690e1 100644
--- a/epan/dissectors/packet-xml.c
+++ b/epan/dissectors/packet-xml.c
@@ -760,7 +760,7 @@ static void add_xml_field(GArray* hfs, int* p_id, gchar* name, gchar* fqn) {
 	hfri.hfinfo.blurb = NULL;
 	hfri.hfinfo.id = 0;
 	hfri.hfinfo.parent = 0;
-	hfri.hfinfo.ref_count = 0;
+	hfri.hfinfo.ref_count = HF_REF_TYPE_NONE;
 	hfri.hfinfo.bitshift = 0;
 	hfri.hfinfo.same_name_next = NULL;
 	hfri.hfinfo.same_name_prev = NULL;
diff --git a/epan/libwireshark.def b/epan/libwireshark.def
index 462350bf28..8adac9928c 100644
--- a/epan/libwireshark.def
+++ b/epan/libwireshark.def
@@ -707,6 +707,7 @@ proto_registrar_is_protocol
 proto_registrar_n
 proto_set_cant_toggle
 proto_set_decoding
+proto_tracking_interesting_fields
 proto_tree_add_bitmask
 proto_tree_add_bits_item
 proto_tree_add_bits_ret_val
diff --git a/epan/proto.c b/epan/proto.c
index 5fd1d02e88..e984c15564 100644
--- a/epan/proto.c
+++ b/epan/proto.c
@@ -101,7 +101,7 @@ wrs_count_bitshift(guint32 bitmask)
 		if(PITEM_FINFO(tree)){					\
 			register header_field_info *hfinfo;		\
 			PROTO_REGISTRAR_GET_NTH(hfindex, hfinfo);	\
-			if((hfinfo->ref_count == 0)			\
+			if((hfinfo->ref_count == HF_REF_TYPE_NONE)	\
 			&& (hfinfo->type!=FT_PROTOCOL)){		\
 				/* just return tree back to the caller */\
 				return tree;				\
@@ -506,19 +506,18 @@ free_GPtrArray_value(gpointer key, gpointer value, gpointer user_data _U_)
 	gint hfid = (gint)(long)key;
 	header_field_info *hfinfo;
 
-
 	PROTO_REGISTRAR_GET_NTH(hfid, hfinfo);
-	if(hfinfo->ref_count){
+	if(hfinfo->ref_count != HF_REF_TYPE_NONE) {
 		/* when a field is referenced by a filter this also
 		   affects the refcount for the parent protocol so we need
 		   to adjust the refcount for the parent as well
 		*/
-		if( (hfinfo->parent != -1) && (hfinfo->ref_count) ){
+		if( hfinfo->parent != -1 ) {
 			header_field_info *parent_hfinfo;
 			PROTO_REGISTRAR_GET_NTH(hfinfo->parent, parent_hfinfo);
-			parent_hfinfo->ref_count -= hfinfo->ref_count;
+			parent_hfinfo->ref_count = HF_REF_TYPE_NONE;
 		}
-		hfinfo->ref_count = 0;
+		hfinfo->ref_count = HF_REF_TYPE_NONE;
 	}
 
 	g_ptr_array_free(ptrs, TRUE);
@@ -527,15 +526,17 @@ free_GPtrArray_value(gpointer key, gpointer value, gpointer user_data _U_)
 static void
 free_node_tree_data(tree_data_t *tree_data)
 {
-        /* Free all the GPtrArray's in the interesting_hfids hash. */
-        g_hash_table_foreach(tree_data->interesting_hfids,
-            free_GPtrArray_value, NULL);
+	if (tree_data->interesting_hfids) {
+		/* Free all the GPtrArray's in the interesting_hfids hash. */
+		g_hash_table_foreach(tree_data->interesting_hfids,
+			free_GPtrArray_value, NULL);
 
-        /* And then destroy the hash. */
-        g_hash_table_destroy(tree_data->interesting_hfids);
+		/* And then destroy the hash. */
+		g_hash_table_destroy(tree_data->interesting_hfids);
+	}
 
-        /* And finally the tree_data_t itself. */
-        g_free(tree_data);
+	/* And finally the tree_data_t itself. */
+	g_free(tree_data);
 }
 
 #define FREE_NODE_FIELD_INFO(finfo)	\
@@ -627,7 +628,7 @@ proto_field_is_referenced(proto_tree *tree, int proto_id)
 		return TRUE;
 
 	PROTO_REGISTRAR_GET_NTH(proto_id, hfinfo);
-	if (hfinfo->ref_count != 0)
+	if (hfinfo->ref_count != HF_REF_TYPE_NONE)
 		return TRUE;
 
 	return FALSE;
@@ -1073,10 +1074,38 @@ get_int_value(tvbuff_t *tvb, gint offset, gint length, gboolean little_endian)
 	return value;
 }
 
+static GPtrArray *proto_lookup_or_create_interesting_hfids(proto_tree *tree,
+                                                     header_field_info	*hfinfo)
+{
+	GPtrArray *ptrs = NULL;
+
+	DISSECTOR_ASSERT(tree);
+	DISSECTOR_ASSERT(hfinfo);
+
+	if (hfinfo->ref_count == HF_REF_TYPE_DIRECT) {
+		if (PTREE_DATA(tree)->interesting_hfids == NULL) {
+			/* Initialize the hash because we now know that it is needed */
+			PTREE_DATA(tree)->interesting_hfids =
+				g_hash_table_new(g_direct_hash, NULL /* g_direct_equal */);
+		}
+
+		ptrs = g_hash_table_lookup(PTREE_DATA(tree)->interesting_hfids,
+								GINT_TO_POINTER(hfinfo->id));
+		if (!ptrs) {
+			/* First element triggers the creation of pointer array */
+			ptrs = g_ptr_array_new();
+			g_hash_table_insert(PTREE_DATA(tree)->interesting_hfids, 
+								GINT_TO_POINTER(hfinfo->id), ptrs);
+		}
+	}
+
+	return ptrs;
+}
+
 /* Add an item to a proto_tree, using the text label registered to that item;
    the item is extracted from the tvbuff handed to it. */
 static proto_item *
-proto_tree_new_item(field_info *new_fi, proto_tree *tree, int hfindex,
+proto_tree_new_item(field_info *new_fi, proto_tree *tree,
     tvbuff_t *tvb, gint start, gint length, gboolean little_endian)
 {
 	proto_item	*pi;
@@ -1084,7 +1113,6 @@ proto_tree_new_item(field_info *new_fi, proto_tree *tree, int hfindex,
 	float		floatval;
 	double		doubleval;
 	char		*string;
-	GHashTable	*hash;
 	GPtrArray	*ptrs;
 
 	/* there is a possibility here that we might raise an exception
@@ -1312,14 +1340,9 @@ proto_tree_new_item(field_info *new_fi, proto_tree *tree, int hfindex,
 
 	/* If the proto_tree wants to keep a record of this finfo
 	 * for quick lookup, then record it. */
-	if (new_fi->hfinfo->ref_count) {
-		/*HERE*/
-		hash = PTREE_DATA(tree)->interesting_hfids;
-		ptrs = g_hash_table_lookup(hash, GINT_TO_POINTER(hfindex));
-		if (ptrs) {
-			g_ptr_array_add(ptrs, new_fi);
-		}
-	}
+	ptrs = proto_lookup_or_create_interesting_hfids(tree, new_fi->hfinfo);
+	if (ptrs)
+		g_ptr_array_add(ptrs, new_fi);
 
 	return pi;
 }
@@ -1358,7 +1381,7 @@ ptvcursor_add(ptvcursor_t *ptvc, int hfindex, gint length,
 	if (new_fi == NULL)
 		return NULL;
 
-	return proto_tree_new_item(new_fi, ptvc->tree, hfindex, ptvc->tvb,
+	return proto_tree_new_item(new_fi, ptvc->tree, ptvc->tvb,
 	    offset, length, little_endian);
 }
 
@@ -1380,7 +1403,7 @@ proto_tree_add_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
 	if (new_fi == NULL)
 		return(NULL);
 
-	return proto_tree_new_item(new_fi, tree, hfindex, tvb, start,
+	return proto_tree_new_item(new_fi, tree, tvb, start,
 	    length, little_endian);
 }
 
@@ -2858,7 +2881,6 @@ proto_tree_add_pi(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start,
 {
 	proto_item	*pi;
 	field_info	*fi;
-	GHashTable	*hash;
 	GPtrArray	*ptrs;
 
 	if (!tree)
@@ -2869,14 +2891,9 @@ proto_tree_add_pi(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start,
 
 	/* If the proto_tree wants to keep a record of this finfo
 	 * for quick lookup, then record it. */
-	if (fi->hfinfo->ref_count) {
-		/*HERE*/
-		hash = PTREE_DATA(tree)->interesting_hfids;
-		ptrs = g_hash_table_lookup(hash, GINT_TO_POINTER(hfindex));
-		if (ptrs) {
-			g_ptr_array_add(ptrs, fi);
-		}
-	}
+	ptrs = proto_lookup_or_create_interesting_hfids(tree, fi->hfinfo);
+	if (ptrs)
+		g_ptr_array_add(ptrs, fi);
 
 	/* Does the caller want to know the fi pointer? */
 	if (pfi) {
@@ -3286,9 +3303,8 @@ proto_tree_create_root(void)
 	pnode->finfo = NULL;
 	pnode->tree_data = g_new(tree_data_t, 1);
 
-	/* Initialize the tree_data_t */
-	pnode->tree_data->interesting_hfids =
-	    g_hash_table_new(g_direct_hash, g_direct_equal);
+	/* Don't initialize the tree_data_t. Wait until we know we need it */
+	pnode->tree_data->interesting_hfids = NULL;
 
 	/* Set the default to FALSE so it's easier to
 	 * find errors; if we expect to see the protocol tree
@@ -3306,18 +3322,15 @@ proto_tree_create_root(void)
 /* "prime" a proto_tree with a single hfid that a dfilter
  * is interested in. */
 void
-proto_tree_prime_hfid(proto_tree *tree, gint hfid)
+proto_tree_prime_hfid(proto_tree *tree _U_, gint hfid)
 {
 	header_field_info *hfinfo;
 
-	g_hash_table_insert(PTREE_DATA(tree)->interesting_hfids,
-		GINT_TO_POINTER(hfid), g_ptr_array_new());
-
 	PROTO_REGISTRAR_GET_NTH(hfid, hfinfo);
 	/* this field is referenced by a filter so increase the refcount.
 	   also increase the refcount for the parent, i.e the protocol.
 	*/
-	hfinfo->ref_count++;
+	hfinfo->ref_count = HF_REF_TYPE_DIRECT;
 	/* only increase the refcount if there is a parent.
 	   if this is a protocol and not a field then parent will be -1
 	   and there is no parent to add any refcounting for.
@@ -3325,7 +3338,12 @@ proto_tree_prime_hfid(proto_tree *tree, gint hfid)
 	if (hfinfo->parent != -1) {
 		header_field_info *parent_hfinfo;
 		PROTO_REGISTRAR_GET_NTH(hfinfo->parent, parent_hfinfo);
-		parent_hfinfo->ref_count++;
+
+		/* Mark parent as indirectly referenced unless it is already directly
+		 * referenced, i.e. the user has specified the parent in a filter.
+		 */
+		if (parent_hfinfo->ref_count != HF_REF_TYPE_DIRECT)
+			parent_hfinfo->ref_count = HF_REF_TYPE_INDIRECT;
 	}
 }
 
@@ -3536,7 +3554,7 @@ proto_register_protocol(const char *name, const char *short_name, const char *fi
     hfinfo->strings = protocol;
     hfinfo->bitmask = 0;
     hfinfo->bitshift = 0;
-    hfinfo->ref_count = 0;
+    hfinfo->ref_count = HF_REF_TYPE_NONE;
     hfinfo->blurb = NULL;
     hfinfo->parent = -1; /* this field differentiates protos and fields */
 
@@ -5013,10 +5031,18 @@ proto_check_for_protocol_or_field(proto_tree* tree, int id)
 GPtrArray*
 proto_get_finfo_ptr_array(proto_tree *tree, int id)
 {
-	return g_hash_table_lookup(PTREE_DATA(tree)->interesting_hfids,
-	    GINT_TO_POINTER(id));
+	if (PTREE_DATA(tree)->interesting_hfids != NULL)
+		return g_hash_table_lookup(PTREE_DATA(tree)->interesting_hfids,
+								GINT_TO_POINTER(id));
+	else
+		return NULL;
 }
 
+gboolean
+proto_tracking_interesting_fields(proto_tree *tree)
+{
+	return (PTREE_DATA(tree)->interesting_hfids != NULL);
+}
 
 /* Helper struct for proto_find_info() and  proto_all_finfos() */
 typedef struct {
diff --git a/epan/proto.h b/epan/proto.h
index 2ef186e1b1..9b7cd29826 100644
--- a/epan/proto.h
+++ b/epan/proto.h
@@ -159,6 +159,12 @@ typedef enum {
 	BASE_CUSTOM	/**< call custom routine (in ->strings) to format */
 } base_display_e;
 
+typedef enum {
+    HF_REF_TYPE_NONE,       /**< Field is not referenced */
+    HF_REF_TYPE_INDIRECT,   /**< Field is indirectly referenced (only applicable for FT_PROTOCOL) via. its child */
+    HF_REF_TYPE_DIRECT,     /**< Field is directly referenced */
+} hf_ref_type;
+
 #define IS_BASE_DUAL(b) ((b)==BASE_DEC_HEX||(b)==BASE_HEX_DEC)
 
 /** information describing a header field */
@@ -181,7 +187,7 @@ struct _header_field_info {
 	/* ------- set by proto routines (prefilled by HFILL macro, see below) ------ */
 	int				id;         /**< Field ID */
 	int				parent;     /**< parent protocol tree */
-	int				ref_count;  /**< is this field referenced by a filter and how often */
+	hf_ref_type			ref_count;  /**< is this field referenced by a filter */
 	int				bitshift;   /**< bits to shift */
 	header_field_info		*same_name_next; /**< Link to next hfinfo with same abbrev */
 	header_field_info		*same_name_prev; /**< Link to previous hfinfo with same abbrev */
@@ -192,7 +198,7 @@ struct _header_field_info {
  * _header_field_info. If new fields are added or removed, it should
  * be changed as necessary.
  */
-#define HFILL 0, 0, 0, 0, NULL, NULL
+#define HFILL 0, 0, HF_REF_TYPE_NONE, 0, NULL, NULL
 
 /** Used when registering many fields at once, using proto_register_field_array() */
 typedef struct hf_register_info {
@@ -1471,6 +1477,12 @@ extern gboolean proto_check_for_protocol_or_field(proto_tree* tree, int id);
  @return GPtrArry pointer */
 extern GPtrArray* proto_get_finfo_ptr_array(proto_tree *tree, int hfindex);
 
+/** Return whether we're tracking any interesting fields.
+    Only works with primed trees, and is fast.
+ @param tree tree of interest
+ @return TRUE if we're tracking interesting fields */
+extern gboolean proto_tracking_interesting_fields(proto_tree *tree);
+
 /** Return GPtrArray* of field_info pointers for all hfindex that appear in
     tree. Works with any tree, primed or unprimed, and is slower than
     proto_get_finfo_ptr_array because it has to search through the tree.
diff --git a/epan/wspython/wspy_proto.c b/epan/wspython/wspy_proto.c
index 5bf673e9ce..2b1dcc6b45 100644
--- a/epan/wspython/wspy_proto.c
+++ b/epan/wspython/wspy_proto.c
@@ -70,7 +70,7 @@ void hf_register_info_add(hf_register_info *hf, guint8 index,
   hf[index].hfinfo.blurb = blurb;
   hf[index].hfinfo.id = 0;
   hf[index].hfinfo.parent = 0;
-  hf[index].hfinfo.ref_count = 0;
+  hf[index].hfinfo.ref_count = HF_REF_TYPE_NONE;
   hf[index].hfinfo.bitshift = 0;
   hf[index].hfinfo.same_name_next = NULL;
   hf[index].hfinfo.same_name_prev = NULL;