From 535f8f7534bde71ebf1e4801dd2e1c31feb0b12a Mon Sep 17 00:00:00 2001 From: Martin Mathieson Date: Fri, 21 Jul 2023 20:54:51 +0000 Subject: Improve check for add_bitmask consistency, and fix up some issues --- epan/dissectors/packet-aim.c | 4 ++-- epan/dissectors/packet-atalk.c | 2 +- epan/dissectors/packet-bthci_cmd.c | 2 +- epan/dissectors/packet-nfs.c | 14 +++++------ tools/check_typed_item_calls.py | 48 ++++++++++++++++++++++++++++---------- 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: -- cgit v1.2.3