Improve check for add_bitmask consistency, and fix up some issues

5 files changed, 47 insertions, 23 deletions

diff --git a/epan/dissectors/packet-aim.c b/epan/dissectors/packet-aim.c
index a82d12ed87..142b61b5f4 100644
--- a/epan/dissectors/packet-aim.c
+++ b/epan/dissectors/packet-aim.c
@@ -2971,8 +2971,8 @@ static const value_string extended_data_message_types[] = {
 };
 
 #define EXTENDED_DATA_MFLAG_NORMAL 0x01
-#define EXTENDED_DATA_MFLAG_AUTO 0x03
-#define EXTENDED_DATA_MFLAG_MULTI 0x80
+#define EXTENDED_DATA_MFLAG_AUTO   0x02
+#define EXTENDED_DATA_MFLAG_MULTI  0x80
 
 #define EVIL_ORIGIN_ANONYMOUS		1
 #define EVIL_ORIGIN_NONANONYMOUS 	2
diff --git a/epan/dissectors/packet-atalk.c b/epan/dissectors/packet-atalk.c
index 4c37709e38..396e7af519 100644
--- a/epan/dissectors/packet-atalk.c
+++ b/epan/dissectors/packet-atalk.c
@@ -1966,7 +1966,7 @@ proto_register_atalk(void)
         NULL, HFILL }},
 
     { &hf_zip_flags,
-      { "Flags",        "zip.flags", FT_UINT8, BASE_HEX, NULL, 0xC0,
+      { "Flags",        "zip.flags", FT_UINT8, BASE_HEX, NULL, 0xE0,
         NULL, HFILL }},
     { &hf_zip_last_flag,
       { "Last Flag",    "zip.last_flag", FT_BOOLEAN, BASE_NONE, NULL, 0x0,
diff --git a/epan/dissectors/packet-bthci_cmd.c b/epan/dissectors/packet-bthci_cmd.c
index 033363d498..d10d7d54f9 100644
--- a/epan/dissectors/packet-bthci_cmd.c
+++ b/epan/dissectors/packet-bthci_cmd.c
@@ -10380,7 +10380,7 @@ proto_register_btcommon(void)
         },
         {&hf_btcommon_eir_ad_ips_flags_reserved,
             {"Reserved", "btcommon.eir_ad.entry.ips.flags.reserved",
-            FT_UINT8, BASE_HEX, NULL, 0xC0,
+            FT_UINT8, BASE_HEX, NULL, 0x80,
             NULL, HFILL}
         },
         {&hf_btcommon_eir_ad_ips_flags_location_name,
diff --git a/epan/dissectors/packet-nfs.c b/epan/dissectors/packet-nfs.c
index 2c738f296d..e01f1abc18 100644
--- a/epan/dissectors/packet-nfs.c
+++ b/epan/dissectors/packet-nfs.c
@@ -7033,11 +7033,11 @@ dissect_nfs4_mode(tvbuff_t *tvb, int offset, proto_tree *tree)
 	return dissect_nfs2_mode(tvb, offset, tree);
 }
 
-#define FH4_PERSISTENT 0x00000000
+#define FH4_PERSISTENT         0x00000000
 #define FH4_NOEXPIRE_WITH_OPEN 0x00000001
-#define FH4_VOLATILE_ANY 0x00000002
-#define FH4_VOL_MIGRATION 0x00000004
-#define FH4_VOL_RENAME 0x00000008
+#define FH4_VOLATILE_ANY       0x00000002
+#define FH4_VOL_MIGRATION      0x00000004
+#define FH4_VOL_RENAME         0x00000008
 
 static const value_string nfs4_fattr4_fh_expire_type_names[] = {
 	{ FH4_PERSISTENT, "FH4_PERSISTENT" },
@@ -12669,15 +12669,15 @@ proto_register_nfs(void)
 		{ &hf_nfs4_fattr_fh_expiry_volatile_any, {
 			"volatile_any", "nfs.fattr4_fh_expire_type.volatile_any",
 			FT_BOOLEAN, 32,
-			NULL, FH4_NOEXPIRE_WITH_OPEN, NULL, HFILL }},
+			NULL, FH4_VOLATILE_ANY, NULL, HFILL }},
 		{ &hf_nfs4_fattr_fh_expiry_vol_migration, {
 			"vol_migration", "nfs.fattr4_fh_expire_type.vol_migration",
 			FT_BOOLEAN, 32,
-			NULL, FH4_NOEXPIRE_WITH_OPEN, NULL, HFILL }},
+			NULL, FH4_VOL_MIGRATION, NULL, HFILL }},
 		{ &hf_nfs4_fattr_fh_expiry_vol_rename, {
 			"vol_rename", "nfs.fattr4_fh_expire_type.vol_rename",
 			FT_BOOLEAN, 32,
-			NULL, FH4_NOEXPIRE_WITH_OPEN, NULL, HFILL }},
+			NULL, FH4_VOL_RENAME, NULL, HFILL }},
 
 		{ &hf_nfs4_fattr_hidden, {
 			"fattr4_hidden", "nfs.fattr4_hidden", FT_BOOLEAN, BASE_NONE,
diff --git a/tools/check_typed_item_calls.py b/tools/check_typed_item_calls.py
index 8f00f5a421..0b0a51d7f0 100755
--- a/tools/check_typed_item_calls.py
+++ b/tools/check_typed_item_calls.py
@@ -160,7 +160,8 @@ class APICheck:
 
 
 
-    def check_against_items(self, items_defined, items_declared, items_declared_extern, check_missing_items=False):
+    def check_against_items(self, items_defined, items_declared, items_declared_extern, check_missing_items=False,
+                            field_arrays=None):
         global errors_found
         global warnings_found
 
@@ -182,6 +183,7 @@ class APICheck:
                           self.file + ':' + str(call.line_number) +
                           ' with length ' + str(call.length) + ' - must be > 0 or -1')
                     errors_found += 1
+
             if call.hf_name in items_defined:
                 # Is type allowed?
                 if not items_defined[call.hf_name].item_type in self.allowed_types:
@@ -197,8 +199,17 @@ class APICheck:
                           ' with mask ' + items_defined[call.hf_name].mask + '    (must be zero!)\n')
                     errors_found += 1
 
+            if self.fun_name.find('add_bitmask') != -1 and call.hf_name in items_defined and field_arrays:
+                if call.fields in field_arrays:
+                    if (items_defined[call.hf_name].mask_value and
+                        field_arrays[call.fields][1] != 0 and items_defined[call.hf_name].mask_value != field_arrays[call.fields][1]):
+                        # TODO: only really a problem if bit is set in array but not in top-level item?
+                        print('Warning:', self.file, call.hf_name, call.fields, "masks don't match. root=",
+                              items_defined[call.hf_name].mask,
+                              "array has", hex(field_arrays[call.fields][1]))
+                        warnings_found += 1
 
-            elif check_missing_items:
+            if check_missing_items:
                 if call.hf_name in items_declared and not call.hf_name in items_declared_extern:
                 #not in common_hf_var_names:
                     print('Warning:', self.file + ':' + str(call.line_number),
@@ -290,7 +301,8 @@ class ProtoTreeAddItemCheck(APICheck):
                                 warnings_found += 1
                         self.calls.append(Call(hf_name, macros, line_number=line_number, length=m.group(2)))
 
-    def check_against_items(self, items_defined, items_declared, items_declared_extern, check_missing_items=False):
+    def check_against_items(self, items_defined, items_declared, items_declared_extern,
+                            check_missing_items=False, field_arrays=None):
         # For now, only complaining if length if call is longer than the item type implies.
         #
         # Could also be bugs where the length is always less than the type allows.
@@ -884,6 +896,7 @@ 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))
+# N.B., proto_tree_add_bitmask_list does not have a root item, just a subtree...
 
 add_bits_types = { 'FT_CHAR', 'FT_BOOLEAN',
                    'FT_UINT8', 'FT_UINT16', 'FT_UINT24', 'FT_UINT32', 'FT_UINT40', 'FT_UINT48', 'FT_UINT56', 'FT_UINT64',
@@ -988,6 +1001,7 @@ def find_items(filename, macros, check_mask=False, mask_exact_width=False, check
 # the 6th arg of ..add_bitmask_..() calls...
 # TODO: return items (rather than local checks) from here so can be checked against list of calls for given filename
 def find_field_arrays(filename, all_fields, all_hf):
+    field_entries = {}
     global warnings_found
     with open(filename, 'r', encoding="utf8") as f:
         contents = f.read()
@@ -999,20 +1013,22 @@ def find_field_arrays(filename, all_fields, all_hf):
         for m in matches:
             name = m.group(1)
             # Ignore if not used in a call to an _add_bitmask_ API
-            if name not in all_fields:
+            if not name in all_fields:
                 continue
-            all_fields = m.group(2)
-            all_fields = all_fields.replace('&', '')
-            all_fields = all_fields.replace(',', '')
+
+            fields_text = m.group(2)
+            fields_text = fields_text.replace('&', '')
+            fields_text = fields_text.replace(',', '')
 
             # Get list of each hf field in the array
-            fields = all_fields.split()
+            fields = fields_text.split()
 
             if fields[0].startswith('ett_'):
                 continue
             if fields[-1].find('NULL') == -1 and fields[-1] != '0':
                 print('Warning:', filename, name, 'is not NULL-terminated - {', ', '.join(fields), '}')
                 warnings_found += 1
+                continue
 
             # Do any hf items reappear?
             seen_fields = set()
@@ -1042,7 +1058,10 @@ def find_field_arrays(filename, all_fields, all_hf):
                         warnings_found += 1
                     set_field_width = new_field_width
 
-    return []
+            # Add entry to table
+            field_entries[name] = (fields[0:-1], combined_mask)
+
+    return field_entries
 
 def find_item_declarations(filename):
     items = set()
@@ -1116,19 +1135,24 @@ def checkFile(filename, check_mask=False, mask_exact_width=False, check_label=Fa
 
     fields = set()
 
-    # Check each API
+    # Get 'fields' out of calls
     for c in apiChecks:
         c.find_calls(filename, macros)
         for call in c.calls:
+            # From _add_bitmask() calls
             if call.fields:
                 fields.add(call.fields)
 
-        c.check_against_items(items_defined, items_declared, items_extern_declared, check_missing_items)
-
     # Checking for lists of fields for add_bitmask calls
+    field_arrays = {}
     if check_bitmask_fields:
         field_arrays = find_field_arrays(filename, fields, items_defined)
 
+    # Now actually check the calls
+    for c in apiChecks:
+        c.check_against_items(items_defined, items_declared, items_extern_declared, check_missing_items, field_arrays)
+
+
     if label_vs_filter:
         matches = 0
         for hf in items_defined: