proto: Add a debug check to sanity-check range_string

Check is enabled by #ifdef ENABLE_CHECK_FILTER
Remaining issues found by this check are fixed here,
along with a documentation note that the entries
are checked in order and the first match is used.

The only issue not yet fixed is in packet-isup.c,
where the spec was not available to me.

Change-Id: Ife747cda9b91a265bc2b81ce0a53f55f3389919e
Reviewed-on: https://code.wireshark.org/review/36708
Petri-Dish: Martin Mathieson <martin.r.mathieson@googlemail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Martin Mathieson <martin.r.mathieson@googlemail.com>

9 files changed, 48 insertions, 14 deletions

diff --git a/doc/README.dissector b/doc/README.dissector
index 8e93e9c8e9..7887b45ac6 100644
--- a/doc/README.dissector
+++ b/doc/README.dissector
@@ -1173,7 +1173,10 @@ For fields of that type, you would declare an array of "range_string"s:
     };
 
 If INTVAL_MIN equals INTVAL_MAX for a given entry the range_string
-behavior collapses to the one of value_string.
+behavior collapses to the one of value_string. Note that each range_string
+within the array is tested in order, so any 'catch-all' entries need to come
+after specific individual entries.
+
 For FT_(U)INT* fields that need a 'range_string' struct, the 'strings' field
 would be set to 'RVALS(rvalstringname)'. Furthermore, 'display' field must be
 ORed with 'BASE_RANGE_STRING' (e.g. BASE_DEC|BASE_RANGE_STRING).
diff --git a/epan/dissectors/asn1/gsm_map/packet-gsm_map-template.c b/epan/dissectors/asn1/gsm_map/packet-gsm_map-template.c
index f788f8f0ff..b5c17ef019 100644
--- a/epan/dissectors/asn1/gsm_map/packet-gsm_map-template.c
+++ b/epan/dissectors/asn1/gsm_map/packet-gsm_map-template.c
@@ -361,7 +361,6 @@ static const range_string gsm_map_na_vals[] = {
   { 3, 3, "national (significant) number (national use)" },
   { 4, 4, "international number" },
   { 5, 0x6f, "spare" },
-  { 0x70, 0x7e, "spare" },
   { 0x70, 0x7e, "reserved for national use" },
   { 0x7f, 0x7f, "spare" },
   { 0, 0, NULL }
diff --git a/epan/dissectors/packet-gsm_map.c b/epan/dissectors/packet-gsm_map.c
index a0aa235786..0b68191577 100644
--- a/epan/dissectors/packet-gsm_map.c
+++ b/epan/dissectors/packet-gsm_map.c
@@ -2963,7 +2963,6 @@ static const range_string gsm_map_na_vals[] = {
   { 3, 3, "national (significant) number (national use)" },
   { 4, 4, "international number" },
   { 5, 0x6f, "spare" },
-  { 0x70, 0x7e, "spare" },
   { 0x70, 0x7e, "reserved for national use" },
   { 0x7f, 0x7f, "spare" },
   { 0, 0, NULL }
@@ -22054,7 +22053,7 @@ dissect_NokiaMAP_Extensions_AllowedServiceData(gboolean implicit_tag _U_, tvbuff
 
 
 /*--- End of included file: packet-gsm_map-fn.c ---*/
-#line 1102 "./asn1/gsm_map/packet-gsm_map-template.c"
+#line 1101 "./asn1/gsm_map/packet-gsm_map-template.c"
 
 /* Specific translation for MAP V3 */
 const value_string gsm_map_V1V2_opr_code_strings[] = {
@@ -22277,7 +22276,7 @@ const value_string gsm_map_opr_code_strings[] = {
 /* Unknown or empty loop list OPERATION */
 
 /*--- End of included file: packet-gsm_map-table.c ---*/
-#line 1113 "./asn1/gsm_map/packet-gsm_map-template.c"
+#line 1112 "./asn1/gsm_map/packet-gsm_map-template.c"
   { 0, NULL }
 };
 
@@ -22495,7 +22494,7 @@ static const value_string gsm_map_err_code_string_vals[] = {
 /* Unknown or empty loop list OPERATION */
 
 /*--- End of included file: packet-gsm_map-table.c ---*/
-#line 1119 "./asn1/gsm_map/packet-gsm_map-template.c"
+#line 1118 "./asn1/gsm_map/packet-gsm_map-template.c"
     { 0, NULL }
 };
 #endif
@@ -32116,7 +32115,7 @@ void proto_register_gsm_map(void) {
         NULL, HFILL }},
 
 /*--- End of included file: packet-gsm_map-hfarr.c ---*/
-#line 3410 "./asn1/gsm_map/packet-gsm_map-template.c"
+#line 3409 "./asn1/gsm_map/packet-gsm_map-template.c"
   };
 
   /* List of subtrees */
@@ -32871,7 +32870,7 @@ void proto_register_gsm_map(void) {
     &ett_NokiaMAP_Extensions_AllowedServiceData,
 
 /*--- End of included file: packet-gsm_map-ettarr.c ---*/
-#line 3449 "./asn1/gsm_map/packet-gsm_map-template.c"
+#line 3448 "./asn1/gsm_map/packet-gsm_map-template.c"
   };
 
   static ei_register_info ei[] = {
@@ -33015,7 +33014,7 @@ void proto_register_gsm_map(void) {
 
 
 /*--- End of included file: packet-gsm_map-dis-tab.c ---*/
-#line 3509 "./asn1/gsm_map/packet-gsm_map-template.c"
+#line 3508 "./asn1/gsm_map/packet-gsm_map-template.c"
   oid_add_from_string("ericsson-gsm-Map-Ext","1.2.826.0.1249.58.1.0" );
   oid_add_from_string("accessTypeNotAllowed-id","1.3.12.2.1107.3.66.1.2");
   /*oid_add_from_string("map-ac networkLocUp(1) version3(3)","0.4.0.0.1.0.1.3" );
diff --git a/epan/dissectors/packet-hislip.c b/epan/dissectors/packet-hislip.c
index 349ae32e68..2caab292c5 100644
--- a/epan/dissectors/packet-hislip.c
+++ b/epan/dissectors/packet-hislip.c
@@ -138,7 +138,7 @@ static const range_string messagetypestring[] =
 {
     { HISLIP_INITIALIZE                     , HISLIP_INITIALIZE                     , "Initialize" },
     { HISLIP_INITIALIZERESPONSE             , HISLIP_INITIALIZERESPONSE             , "InitializeResponse" },
-    { HISLIP_FATALERROR                     , HISLIP_ERROR                          , "FatalError" },
+    { HISLIP_FATALERROR                     , HISLIP_FATALERROR                     , "FatalError" },
     { HISLIP_ERROR                          , HISLIP_ERROR                          , "Error" },
     { HISLIP_ASYNCLOCK                      , HISLIP_ASYNCLOCK                      , "AsyncLock" },
     { HISLIP_ASYNCLOCK_RESPONSE             , HISLIP_ASYNCLOCK_RESPONSE             , "AsyncLockResponse" },
diff --git a/epan/dissectors/packet-ieee1722.c b/epan/dissectors/packet-ieee1722.c
index ec6a3110bb..82f897104d 100644
--- a/epan/dissectors/packet-ieee1722.c
+++ b/epan/dissectors/packet-ieee1722.c
@@ -317,7 +317,7 @@ static const range_string subtype_range_rvals[] = {
     { 0x7f, 0x7f,   "Experimental Format Stream" },
     { 0x80, 0x81,   "Reserved for future protocols" },
     { 0x82, 0x82,   "Non Time Synchronous Control Format" },
-    { 0x83, 0xed,   "Reserved for future protocols" },
+    { 0x83, 0xeb,   "Reserved for future protocols" },
     { 0xec, 0xec,   "ECC Signed Control Format" },
     { 0xed, 0xed,   "ECC Encrypted Control Format" },
     { 0xee, 0xee,   "AES Encrypted Format Discrete" },
diff --git a/epan/dissectors/packet-isup.c b/epan/dissectors/packet-isup.c
index 5288047b4a..148adbe2b8 100644
--- a/epan/dissectors/packet-isup.c
+++ b/epan/dissectors/packet-isup.c
@@ -7531,7 +7531,7 @@ dissect_japan_isup_carrier_information(tvbuff_t *parameter_tvb, packet_info *pin
 static const range_string japan_isup_charge_delay_type_value[] = {
   {  0,    0,    "Spare" },
   {  1,    0xfc, "Reserved for network specific use" },
-  {  0x81, 0xfa, "Spare" },
+  {  0x81, 0xfa, "Spare" },                         /* TODO: overlaps with previous, need to check (non-free) spec */
   {  0xfd, 0xfd, "Charge rate transfer" },
   {  0xfe, 0xfe, "Terminating charge area information" },
   {  0xff, 0xff, "Spare" },
diff --git a/epan/dissectors/packet-optommp.c b/epan/dissectors/packet-optommp.c
index c29b321352..6d41ce65c8 100644
--- a/epan/dissectors/packet-optommp.c
+++ b/epan/dissectors/packet-optommp.c
@@ -169,7 +169,7 @@ static const range_string optommp_mm_areas[] = {
             "Email Configuration - Read/Write" },
     {   0xf1540000, 0xf1540efc,
             "Serial Event Configuration - Read/Write" },
-    {   0xf1560000, 0xf1f60f7f,
+    {   0xf1560000, 0xf1560f7f,
             "Wiegand Serial Event Configuration - Read/Write" },
     {   0xf1808000, 0xf1809ffe,
             "SNAP High-Density Digital - Read Only" },
diff --git a/epan/dissectors/packet-tn5250.c b/epan/dissectors/packet-tn5250.c
index 841e695ee3..e3aca3879d 100644
--- a/epan/dissectors/packet-tn5250.c
+++ b/epan/dissectors/packet-tn5250.c
@@ -1740,7 +1740,7 @@ static const range_string vals_tn5250_dpt_id[] = {
 #define CORE_AREA_COMMAND_KEYS  0x80
 
 static const range_string vals_tn5250_dfdpck_data_field[] = {
-  { 0x00,  0x40, "Invalid Data Field Type"},
+  { 0x00,  0x3f, "Invalid Data Field Type"},
   { 0x40,  0x40, "Top Row Command Key Functions"},
   { 0x41,  0x79, "Invalid Data Field Type"},
   { 0x80,  0x80, "Core Area Key Command Functions"},
diff --git a/epan/proto.c b/epan/proto.c
index f0110e8972..3bce390288 100644
--- a/epan/proto.c
+++ b/epan/proto.c
@@ -8056,6 +8056,39 @@ tmp_fld_check_assert(header_field_info *hfinfo)
 			}
 		}
 	}
+
+	if (hfinfo->display & BASE_RANGE_STRING) {
+		const range_string *rs = (const range_string*)(hfinfo->strings);
+		if (rs) {
+			const range_string *this_it = rs;
+
+			do {
+				if (this_it->value_max < this_it->value_min) {
+					g_warning("value_range_string error:  %s (%s) entry for \"%s\" - max(%u 0x%x) is less than min(%u 0x%x)\n",
+							  hfinfo->name, hfinfo->abbrev,
+							  this_it->strptr,
+							  this_it->value_max, this_it->value_max,
+							  this_it->value_min, this_it->value_min);
+					++this_it;
+					continue;
+				}
+
+				for (const range_string *prev_it=rs; prev_it < this_it; ++prev_it) {
+					/* Not OK if this one is completely hidden by an earlier one! */
+					if ((prev_it->value_min <= this_it->value_min) && (prev_it->value_max >= this_it->value_max)) {
+						g_warning("value_range_string error:  %s (%s) hidden by earlier entry "
+								  "(prev=\"%s\":  %u 0x%x -> %u 0x%x)  (this=\"%s\":  %u 0x%x -> %u 0x%x)\n",
+								  hfinfo->name, hfinfo->abbrev,
+								  prev_it->strptr, prev_it->value_min, prev_it->value_min,
+								  prev_it->value_max, prev_it->value_max,
+								  this_it->strptr, this_it->value_min, this_it->value_min,
+								  this_it->value_max, this_it->value_max);
+					}
+				}
+				++this_it;
+			} while (this_it->strptr);
+		}
+	}
 #endif
 
 	switch (hfinfo->type) {