diff options
author | stig <stig@f5534014-38df-0310-8fa8-9805f1628bb7> | 2009-07-18 20:10:14 +0000 |
---|---|---|
committer | stig <stig@f5534014-38df-0310-8fa8-9805f1628bb7> | 2009-07-18 20:10:14 +0000 |
commit | 484de73a117bbf8f9fd2a5c0d41e93cb411a4a39 (patch) | |
tree | 6f8332c29a1b5eeb2d843d99c1280199364d3b63 | |
parent | 3de2e2a698946362f9f922a2a56add75fbbd9d2a (diff) |
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.
git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@29137 f5534014-38df-0310-8fa8-9805f1628bb7
-rw-r--r-- | epan/dissectors/packet-xml.c | 2 | ||||
-rw-r--r-- | epan/libwireshark.def | 1 | ||||
-rw-r--r-- | epan/proto.c | 120 | ||||
-rw-r--r-- | epan/proto.h | 16 | ||||
-rw-r--r-- | epan/wspython/wspy_proto.c | 2 | ||||
-rw-r--r-- | plugins/mate/mate_runtime.c | 18 |
6 files changed, 99 insertions, 60 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; diff --git a/plugins/mate/mate_runtime.c b/plugins/mate/mate_runtime.c index 5d48bf174d..6d5684dec6 100644 --- a/plugins/mate/mate_runtime.c +++ b/plugins/mate/mate_runtime.c @@ -36,7 +36,7 @@ struct _mate_range { typedef struct _tmp_pdu_data { GPtrArray* ranges; - GHashTable* interesting; + proto_tree* tree; mate_pdu* pdu; } tmp_pdu_data; @@ -676,7 +676,7 @@ static void get_pdu_fields(gpointer k, gpointer v, gpointer p) { gchar* s; - fis = (GPtrArray*) g_hash_table_lookup(data->interesting,GINT_TO_POINTER(hfid)); + fis = proto_get_finfo_ptr_array(data->tree, hfid); if (fis) { for (i = 0; i < fis->len; i++) { @@ -711,7 +711,7 @@ static void get_pdu_fields(gpointer k, gpointer v, gpointer p) { } } -static mate_pdu* new_pdu(mate_cfg_pdu* cfg, guint32 framenum, field_info* proto, GHashTable* interesting) { +static mate_pdu* new_pdu(mate_cfg_pdu* cfg, guint32 framenum, field_info* proto, proto_tree* tree) { mate_pdu* pdu = g_mem_chunk_alloc(rd->mate_items); field_info* cfi; GPtrArray* ptrs; @@ -748,7 +748,7 @@ static mate_pdu* new_pdu(mate_cfg_pdu* cfg, guint32 framenum, field_info* proto, data.ranges = g_ptr_array_new(); data.pdu = pdu; - data.interesting = interesting; + data.tree = tree; /* first we create the proto range */ proto_range = g_malloc(sizeof(mate_range)); @@ -763,7 +763,7 @@ static mate_pdu* new_pdu(mate_cfg_pdu* cfg, guint32 framenum, field_info* proto, /* we move forward in the tranport */ for (i = cfg->transport_ranges->len; i--; ) { hfid = *((int*)g_ptr_array_index(cfg->transport_ranges,i)); - ptrs = (GPtrArray*) g_hash_table_lookup(interesting,GINT_TO_POINTER(hfid)); + ptrs = proto_get_finfo_ptr_array(tree, hfid); min_dist = 99999; range_fi = NULL; @@ -799,7 +799,7 @@ static mate_pdu* new_pdu(mate_cfg_pdu* cfg, guint32 framenum, field_info* proto, for (i = 0 ; i < cfg->payload_ranges->len; i++) { hfid = *((int*)g_ptr_array_index(cfg->payload_ranges,i)); - ptrs = (GPtrArray*) g_hash_table_lookup(interesting,GINT_TO_POINTER(hfid)); + ptrs = proto_get_finfo_ptr_array(tree, hfid); min_dist = 99999; range_fi = NULL; @@ -853,14 +853,14 @@ extern void mate_analyze_frame(packet_info *pinfo, proto_tree* tree) { rd->now = (float) nstime_to_sec(&pinfo->fd->rel_ts); - if ( tree->tree_data && tree->tree_data->interesting_hfids + if ( proto_tracking_interesting_fields(tree) && rd->highest_analyzed_frame < pinfo->fd->num ) { for ( i = 0; i < mc->pducfglist->len; i++ ) { cfg = g_ptr_array_index(mc->pducfglist,i); dbg_print (dbg_pdu,4,dbg_facility,"mate_analyze_frame: trying to extract: %s",cfg->name); - protos = (GPtrArray*) g_hash_table_lookup(tree->tree_data->interesting_hfids,GINT_TO_POINTER(cfg->hfid_proto)); + protos = proto_get_finfo_ptr_array(tree, cfg->hfid_proto); if (protos) { pdu = NULL; @@ -870,7 +870,7 @@ extern void mate_analyze_frame(packet_info *pinfo, proto_tree* tree) { dbg_print (dbg_pdu,3,dbg_facility,"mate_analyze_frame: found matching proto, extracting: %s",cfg->name); proto = (field_info*) g_ptr_array_index(protos,j); - pdu = new_pdu(cfg, pinfo->fd->num, proto, tree->tree_data->interesting_hfids); + pdu = new_pdu(cfg, pinfo->fd->num, proto, tree); if (cfg->criterium) { criterium_match = new_avpl_from_match(cfg->criterium_match_mode,"",pdu->avpl,cfg->criterium,FALSE); |