check_typed_item_calls.py: check for consecutive calls to same item

7 files changed, 59 insertions, 14 deletions

diff --git a/epan/dissectors/packet-gsm_sim.c b/epan/dissectors/packet-gsm_sim.c
index f3567f9c19..f5cab847f7 100644
--- a/epan/dissectors/packet-gsm_sim.c
+++ b/epan/dissectors/packet-gsm_sim.c
@@ -1458,7 +1458,6 @@ dissect_rsp_apdu_tvb(tvbuff_t *tvb, gint offset, packet_info *pinfo, proto_tree
 
 	/* obtain status word */
 	sw = tvb_get_ntohs(tvb, offset);
-	/* proto_tree_add_item(sim_tree, hf_apdu_sw, tvb, offset, 2, ENC_BIG_ENDIAN); */
 	proto_tree_add_uint_format(sim_tree, hf_apdu_sw, tvb, offset, 2, sw,
 							"Status Word: %04x %s", sw, get_sw_string(sw));
 	offset += 2;
diff --git a/epan/dissectors/packet-ieee80211.c b/epan/dissectors/packet-ieee80211.c
index 82e526bf4e..5c8e355627 100644
--- a/epan/dissectors/packet-ieee80211.c
+++ b/epan/dissectors/packet-ieee80211.c
@@ -32947,6 +32947,7 @@ dissect_ieee80211_pv1(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree,
     const gchar *ether_name = tvb_get_ether_name(tvb, offset);
 
     set_src_addr_cols(pinfo, tvb, offset, "TA");
+    /* TODO: why adding as visible *and* hidden? */
     proto_tree_add_item(hdr_tree, hf_ieee80211_addr_ta, tvb, offset, 6, ENC_NA);
     hidden = proto_tree_add_item(hdr_tree, hf_ieee80211_addr_ta, tvb, offset, 6, ENC_NA);
     PROTO_ITEM_SET_HIDDEN(hidden);
diff --git a/epan/dissectors/packet-kafka.c b/epan/dissectors/packet-kafka.c
index e211288609..f60ecda88c 100644
--- a/epan/dissectors/packet-kafka.c
+++ b/epan/dissectors/packet-kafka.c
@@ -9135,10 +9135,6 @@ dissect_kafka(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U
 
         /* Also show api version from request */
         ti = proto_tree_add_int(kafka_tree, hf_kafka_response_api_version, tvb,
-                0, 0, matcher->api_version);
-        proto_item_set_generated(ti);
-        proto_item_set_hidden(ti);
-        ti = proto_tree_add_int(kafka_tree, hf_kafka_response_api_version, tvb,
                                 0, 0, matcher->api_version);
         proto_item_set_generated(ti);
         kafka_check_supported_api_version(pinfo, ti, matcher);
diff --git a/epan/dissectors/packet-m2ua.c b/epan/dissectors/packet-m2ua.c
index bb42be62f2..273996cb2f 100644
--- a/epan/dissectors/packet-m2ua.c
+++ b/epan/dissectors/packet-m2ua.c
@@ -884,10 +884,6 @@ dissect_parameter(tvbuff_t *parameter_tvb, packet_info *pinfo, proto_tree *tree,
   if ((protocol_data_1_global == PROTOCOL_DATA_1_DRAFT_7) &&
       (tag == PROTOCOL_DATA_1_DRAFT_7))
   {
-     proto_item *hidden_item;
-     hidden_item = proto_tree_add_uint(parameter_tree, hf_parameter_tag, parameter_tvb, PARAMETER_TAG_OFFSET, PARAMETER_TAG_LENGTH, tag);
-     proto_item_set_hidden(hidden_item);
-
      /* add tag and length to the m2ua tree */
      proto_tree_add_uint_format_value(parameter_tree, hf_parameter_tag, parameter_tvb, PARAMETER_TAG_OFFSET, PARAMETER_TAG_LENGTH,
                                       tag, "Protocol data 1 (0x000e)");
diff --git a/epan/dissectors/packet-mih.c b/epan/dissectors/packet-mih.c
index 78920b2558..7754509cd9 100644
--- a/epan/dissectors/packet-mih.c
+++ b/epan/dissectors/packet-mih.c
@@ -2006,6 +2006,7 @@ static int dissect_mih(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void
         mih_tree = proto_item_add_subtree(ti, ett_mih);
         if(mih_tree)
         {
+                /* TODO: should have a different hf item for this version root */
                 item = proto_tree_add_item(mih_tree, hf_mih_version, tvb, offset, 1, ENC_BIG_ENDIAN);
 
                 ver_flags_tree = proto_item_add_subtree(item, ett_ver_flags);
diff --git a/epan/dissectors/packet-mtp3.c b/epan/dissectors/packet-mtp3.c
index 998fcdd28d..a14bb4c2af 100644
--- a/epan/dissectors/packet-mtp3.c
+++ b/epan/dissectors/packet-mtp3.c
@@ -463,6 +463,7 @@ dissect_mtp3_3byte_pc(tvbuff_t *tvb, guint offset, proto_tree *tree, gint ett_pc
   proto_item_append_text(pc_item, " (%s)", pc_string);
   hidden_item = proto_tree_add_string(tree, hf_pc_string, tvb, offset, ANSI_PC_LENGTH, pc_string);
   proto_item_set_hidden(hidden_item);
+
   snprintf(pc_string, sizeof(pc_string), "0x%x", pc);
   proto_item_append_text(pc_item, " (%s)", pc_string);
   hidden_item = proto_tree_add_string(tree, hf_pc_string, tvb, offset, ANSI_PC_LENGTH, pc_string);
diff --git a/tools/check_typed_item_calls.py b/tools/check_typed_item_calls.py
index 9e839ec2bb..af4aecaeb5 100755
--- a/tools/check_typed_item_calls.py
+++ b/tools/check_typed_item_calls.py
@@ -95,7 +95,9 @@ class APICheck:
                 if to_check.find(self.fun_name) != -1:
                     # Ok, add the next file lines before trying RE
                     for i in range(1, 4):
-                        if line_number+i < total_lines:
+                        if to_check.find(';') != -1:
+                            break
+                        elif line_number+i < total_lines:
                             to_check += (lines[line_number-1+i] + '\n')
                     m = self.p.search(to_check)
                     if m:
@@ -188,11 +190,13 @@ class ProtoTreeAddItemCheck(APICheck):
             for line_number,line in enumerate(lines):
                 # Want to check this, and next few lines
                 to_check = lines[line_number-1] + '\n'
-                # Nothing to check if function name isn't in itk)
+                # Nothing to check if function name isn't in it
                 if to_check.find(self.fun_name) != -1:
                     # Ok, add the next file lines before trying RE
                     for i in range(1, 5):
-                        if line_number+i < total_lines:
+                        if to_check.find(';') != -1:
+                            break
+                        elif line_number+i < total_lines:
                             to_check += (lines[line_number-1+i] + '\n')
                     m = self.p.search(to_check)
                     if m:
@@ -460,6 +464,45 @@ class Item:
                 warnings_found += 1
 
 
+class CombinedCallsCheck:
+    def __init__(self, file, apiChecks):
+        self.file = file
+        self.apiChecks = apiChecks
+        self.get_all_calls()
+
+    def get_all_calls(self):
+        self.all_calls = []
+        # Combine calls into one list.
+        for check in self.apiChecks:
+            self.all_calls += check.calls
+
+        # Sort by line number.
+        self.all_calls.sort(key=lambda x:x.line_number)
+
+    def check_consecutive_item_calls(self):
+        lines = open(self.file, 'r').read().splitlines()
+
+        prev = None
+        for call in self.all_calls:
+            if prev and call.hf_name == prev.hf_name:
+                # More compelling if close together..
+                if call.line_number>prev.line_number and call.line_number-prev.line_number <= 4:
+                    scope_different = False
+                    for l in range(prev.line_number, call.line_number-1):
+                        if lines[l].find('{') != -1 or lines[l].find('}') != -1 or lines[l].find('else') != -1 or lines[l].find('break;') != -1 or lines[l].find('if ') != -1:
+                            scope_different = True
+                            break
+                    # Also more compelling if check for and scope changes { } in lines in-between?
+                    if not scope_different:
+                        print('Warning:', f + ':' + str(call.line_number),
+                              call.hf_name + ' called consecutively at line', call.line_number, '- previous at', prev.line_number)
+                        global warnings_found
+                        warnings_found += 1
+            prev = call
+
+
+
+
 # These are APIs in proto.c that check a set of types at runtime and can print '.. is not of type ..' to the console
 # if the type is not suitable.
 apiChecks = []
@@ -506,7 +549,8 @@ apiChecks.append(APICheck('proto_tree_add_item_ret_varint', { 'FT_INT8', 'FT_INT
 apiChecks.append(APICheck('proto_tree_add_boolean_bits_format_value', { 'FT_BOOLEAN'}))
 apiChecks.append(APICheck('proto_tree_add_boolean_bits_format_value64', { 'FT_BOOLEAN'}))
 apiChecks.append(APICheck('proto_tree_add_ascii_7bits_item', { 'FT_STRING'}))
-apiChecks.append(APICheck('proto_tree_add_checksum', { 'FT_UINT8', 'FT_UINT16', 'FT_UINT24', 'FT_UINT32'}))
+# TODO: positions are different, and takes 2 hf_fields..
+#apiChecks.append(APICheck('proto_tree_add_checksum', { 'FT_UINT8', 'FT_UINT16', 'FT_UINT24', 'FT_UINT32'}))
 apiChecks.append(APICheck('proto_tree_add_int64_bits_format_value', { 'FT_INT40', 'FT_INT48', 'FT_INT56', 'FT_INT64'}))
 
 # TODO: add proto_tree_add_bytes_item, proto_tree_add_time_item ?
@@ -524,7 +568,8 @@ apiChecks.append(APICheck('proto_tree_add_bitmask_with_flags_ret_uint64', bitmas
 apiChecks.append(APICheck('proto_tree_add_bitmask_value', bitmask_types))
 apiChecks.append(APICheck('proto_tree_add_bitmask_value_with_flags', bitmask_types))
 apiChecks.append(APICheck('proto_tree_add_bitmask_len', bitmask_types))
-apiChecks.append(APICheck('proto_tree_add_bitmask_text', bitmask_types))
+# TODO: doesn't even have an hf_item !
+#apiChecks.append(APICheck('proto_tree_add_bitmask_text', bitmask_types))
 
 # Check some ptvcuror calls too.
 apiChecks.append(APICheck('ptvcursor_add_ret_uint', { 'FT_CHAR', 'FT_UINT8', 'FT_UINT16', 'FT_UINT24', 'FT_UINT32'}))
@@ -760,6 +805,12 @@ for f in files:
     checkFile(f, check_mask=args.mask, check_label=args.label,
               check_consecutive=args.consecutive, check_missing_items=args.missing_items)
 
+    # Do checks against all calls.
+    if args.consecutive:
+        combined_calls = CombinedCallsCheck(f, apiChecks)
+        combined_calls.check_consecutive_item_calls()
+
+
 # Show summary.
 print(warnings_found, 'warnings')
 if errors_found: