diff --git a/epan/dissectors/packet-umts_fp.c b/epan/dissectors/packet-umts_fp.c index 1cc0942ea8..a3a6e0600c 100644 --- a/epan/dissectors/packet-umts_fp.c +++ b/epan/dissectors/packet-umts_fp.c @@ -312,12 +312,6 @@ static const value_string frame_type_vals[] = { { 0, NULL } }; -static const value_string direction_vals[] = { - { 0, "Downlink" }, - { 1, "Uplink" }, - { 0, NULL } -}; - static const value_string crci_vals[] = { { 0, "Correct" }, { 1, "Not correct" }, @@ -5911,7 +5905,7 @@ dissect_fp_common(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *dat } /* Add link direction as a generated field */ - ti = proto_tree_add_uint(fp_tree, hf_fp_direction, tvb, 0, 0, p_fp_info->is_uplink); + ti = proto_tree_add_boolean(fp_tree, hf_fp_direction, tvb, 0, 0, p_fp_info->is_uplink); proto_item_set_generated(ti); /* Don't currently handle IuR-specific formats, but it's useful to even see @@ -6106,7 +6100,7 @@ void proto_register_fp(void) }, { &hf_fp_direction, { "Direction", - "fp.direction", FT_UINT8, BASE_HEX, VALS(direction_vals), 0x0, + "fp.direction", FT_BOOLEAN, 8, TFS(&tfs_uplink_downlink), 0x0, "Link direction", HFILL } }, diff --git a/tools/check_tfs.py b/tools/check_tfs.py index b1ad33e54e..19d0af0e9e 100755 --- a/tools/check_tfs.py +++ b/tools/check_tfs.py @@ -16,7 +16,7 @@ import signal # TODO: # - check how many of the definitions in epan/tfs.c are used in other dissectors -# - see if there are other values that should be in epan/tfs.c and shared +# - although even if unused, might be in external dissectors? # Try to exit soon after Ctrl-C is pressed. @@ -30,6 +30,37 @@ def signal_handler(sig, frame): signal.signal(signal.SIGINT, signal_handler) +# Test for whether the given file was automatically generated. +def isGeneratedFile(filename): + # Open file + f_read = open(os.path.join(filename), 'r') + lines_tested = 0 + for line in f_read: + # The comment to say that its generated is near the top, so give up once + # get a few lines down. + if lines_tested > 10: + f_read.close() + return False + if (line.find('Generated automatically') != -1 or + line.find('Generated Automatically') != -1 or + line.find('Autogenerated from') != -1 or + line.find('is autogenerated') != -1 or + line.find('automatically generated by Pidl') != -1 or + line.find('Created by: The Qt Meta Object Compiler') != -1 or + line.find('This file was generated') != -1 or + line.find('This filter was automatically generated') != -1 or + line.find('This file is auto generated, do not edit!') != -1 or + line.find('This file is auto generated') != -1): + + f_read.close() + return True + lines_tested = lines_tested + 1 + + # OK, looks like a hand-written file! + f_read.close() + return False + + # Keep track of custom entries that might appear in multiple dissectors, # so we can consider adding them to tfs.c custom_tfs_entries = {} @@ -59,13 +90,50 @@ class TFS: return '{' + '"' + self.val1 + '", "' + self.val2 + '"}' +class ValueString: + def __init__(self, file, name, vals): + self.file = file + self.name = name + self.raw_vals = vals + self.parsed_vals = {} + self.looks_like_tfs = True + + no_lines = self.raw_vals.count('{') + if no_lines != 3: + self.looks_like_tfs = False + return + + # Now parse out each entry in the value_string + matches = re.finditer(r'\{([\"a-zA-Z\s\d\,]*)\}', self.raw_vals) + for m in matches: + entry = m[1] + # Check each entry looks like part of a TFS entry. + match = re.match(r'\s*([01])\,\s*\"([a-zA-Z\d\s]*\s*)\"', entry) + if match: + if match[1] == '1': + self.parsed_vals[True] = match[2] + else: + self.parsed_vals[False] = match[2] + + # Now have both entries + if len(self.parsed_vals) == 2: + break + else: + self.looks_like_tfs = False + break + + def __str__(self): + return '{' + '"' + self.raw_vals + '"}' + + + def removeComments(code_string): code_string = re.sub(re.compile(r"/\*.*?\*/",re.DOTALL ) ,"" ,code_string) # C-style comment code_string = re.sub(re.compile(r"//.*?\n" ) ,"" ,code_string) # C++-style comment return code_string -# Look for hf items in a dissector file. +# Look for true_false_string items in a dissector file. def findItems(filename): items = {} @@ -86,6 +154,31 @@ def findItems(filename): return items +# Look for value_string entries in a dissector file. +def findValueStrings(filename): + items = {} + + #static const value_string radio_type_vals[] = + #{ + # { 0, "FDD"}, + # { 1, "TDD"}, + # { 0, NULL } + #}; + + with open(filename, 'r') as f: + contents = f.read() + + # Remove comments so as not to trip up RE. + contents = removeComments(contents) + + matches = re.finditer(r'.*const value_string\s*([a-zA-Z0-9_]*)\s*\[\s*\]\s*\=\s*\{([\{\}\d\,a-zA-Z\s\"]*)\};', contents) + for m in matches: + name = m.group(1) + vals = m.group(2) + items[name] = ValueString(filename, name, vals) + + return items + def is_dissector_file(filename): @@ -104,11 +197,13 @@ def findDissectorFilesInFolder(folder): files.append(filename) return files + + warnings_found = 0 errors_found = 0 # Check the given dissector file. -def checkFile(filename, tfs_items, look_for_common=False): +def checkFile(filename, tfs_items, look_for_common=False, check_value_strings=False): global warnings_found global errors_found @@ -145,7 +240,7 @@ def checkFile(filename, tfs_items, look_for_common=False): found = True if found: - print(filename, i, "- could have used", t, 'from tfs.c instead: ', tfs_items[t], + print("Error:" if exact_case else "Warn: ", filename, i, "- could have used", t, 'from tfs.c instead: ', tfs_items[t], '' if exact_case else ' (capitalisation differs)') if exact_case: errors_found += 1 @@ -156,6 +251,44 @@ def checkFile(filename, tfs_items, look_for_common=False): if look_for_common: AddCustomEntry(items[i].val1, items[i].val2, filename) + if check_value_strings: + vs = findValueStrings(filename) + for v in vs: + if vs[v].looks_like_tfs: + found = False + exact_case = False + + #print('Candidate', v, vs[v]) + for t in tfs_items: + found = False + + # + # Do not do this check for plugins; plugins cannot import + # data values from libwireshark (functions, yes; data + # values, no). + # + # Test whether there's a common prefix for the file name + # and "plugin/epan/"; if so, this is a plugin, and there + # is no common path and os.path.commonprefix returns an + # empty string, otherwise it returns the common path, so + # we check whether the common path is an empty string. + # + if os.path.commonprefix([filename, 'plugin/epan/']) == '': + exact_case = False + if tfs_items[t].val1 == vs[v].parsed_vals[True] and tfs_items[t].val2 == vs[v].parsed_vals[False]: + found = True + exact_case = True + elif tfs_items[t].val1.upper() == vs[v].parsed_vals[True].upper() and tfs_items[t].val2.upper() == vs[v].parsed_vals[False].upper(): + found = True + + if found: + print("Warn:" if exact_case else "Note:", filename, 'value_string', v, "- could have used", t, 'from tfs.c instead: ', tfs_items[t], + '' if exact_case else ' (capitalisation differs)') + if exact_case: + warnings_found += 1 + break + + ################################################################# # Main logic. @@ -169,6 +302,9 @@ parser.add_argument('--commits', action='store', help='last N commits to check') parser.add_argument('--open', action='store_true', help='check open files') +parser.add_argument('--check-value-strings', action='store_true', + help='check whether value_strings could have been tfs?') + parser.add_argument('--common', action='store_true', help='check for potential new entries for tfs.c') @@ -234,7 +370,8 @@ tfs_entries = findItems(os.path.join('epan', 'tfs.c')) for f in files: if should_exit: exit(1) - checkFile(f, tfs_entries, look_for_common=args.common) + if not isGeneratedFile(f): + checkFile(f, tfs_entries, look_for_common=args.common, check_value_strings=args.check_value_strings) # Show summary.