Wireshark · Wireshark-dev: [Wireshark-dev] [PATCH] range_string and OSPF bcmodelid

["Francesco Fondelli" <francesco.fondelli@xxxxxxxxx>]

Hi all,

I needed to dissect a simple hf item in ospf, a integer one.
The possible values were in ranges like:

0     3 "foo"
1     4 "bar"
5     5 "shark"
6   100 "reserved"
101 200 "private use"

in such a case I normally use "switch" or "if" statements
and several proto_tree_add_uint_format() or
proto_tree_add_text() to properly handle the "reserved" and
"private use" cases. If I'm not wrong there is no
way to dissect such a hf in one shot using a
single proto_tree_add_xxx() call.

So I defined a range_string struct. It's like value_string
but stores range <-> string pairs.
Moreover I wrote rval_to_str(), match_strrval_idx()
match_strrval() which are behaving exactly as
val_to_str(), match_strval_idx() and match_strval().
(almost cut && paste :-))

now we can do something like:

static const range_string rvals[] = {
       {   0,   3, "foo"},
       {   1,   4, "bar"},
       {   5,   5, "shark" },
       {   6, 100, "reserved"},
       { 101, 200, "private"},
       { 0,     0, NULL}
};

proto_tree_add_uint_format(tree,
                          hf_augh,
                          tvb,
                          offset,
                          1,
                          value,
                          "bla bla: %u (%s)",
                          value,
                          rval_to_str(value, rvals, "Unknown"));

This way people can write less lines of code (yes I'm lazy ;-)))
and IMHO it's more readable.

if you like this solution please apply
"range_string_and_ospf_bcmodel_id.patch",
if you don't please apply "ospf_bcmodel_id.patch". You
find both in attachment, they are diffed against
svn rev. 19892.


Thanks
Ciao
FF

Index: AUTHORS
===================================================================
--- AUTHORS	(revision 19892)
+++ AUTHORS	(working copy)
@@ -2277,6 +2277,7 @@
 	MPLS OAM support, Y.1711
 	RSVP/OSPF Extensions for Support of Diffserv-aware MPLS-TE, RFC 4124
 	Linux Packet Generator support
+	rval_to_str() and alike
 }
 
 Bill Meier		<wmeier [AT] newsguy.com> {
Index: epan/value_string.c
===================================================================
--- epan/value_string.c	(revision 19892)
+++ epan/value_string.c	(working copy)
@@ -109,3 +109,50 @@
   g_snprintf(p, 1024-(p-buf), fmt, val_to_str((val & mask) >> shift, tab, "Unknown"));
   return buf;
 }
+
+
+/* FF: ranges aware versions */
+
+/* Tries to match val against each range in the range_string array rs.
+   Returns the associated string ptr on a match.
+   Formats val with fmt, and returns the resulting string, on failure. */
+const gchar *rval_to_str(guint32 val, const range_string *rs, const char *fmt) 
+{
+  const gchar *ret = NULL;
+
+  g_assert(fmt != NULL);
+
+  ret = match_strrval(val, rs);
+  if(ret != NULL)
+    return ret;
+
+  return ep_strdup_printf(fmt, val);
+}
+
+/* Tries to match val against each range in the range_string array rs.
+   Returns the associated string ptr, and sets "*idx" to the index in
+   that table, on a match, and returns NULL, and sets "*idx" to -1,
+   on failure. */
+const gchar *match_strrval_idx(guint32 val, const range_string *rs, gint *idx)
+{
+  gint i = 0;
+
+  while(rs[i].strptr) {
+    if( (val >= rs[i].value_min) && (val <= rs[i].value_max) ) {
+      *idx = i;
+      return (rs[i].strptr);
+    }
+    i++;
+  }
+
+  *idx = -1;
+  return (NULL);
+}
+
+/* Like match_strrval_idx(), but doesn't return the index. */
+const gchar *match_strrval(guint32 val, const range_string *rs)
+{
+    gint ignore_me = 0;
+    return match_strrval_idx(val, rs, &ignore_me);
+}
+
Index: epan/value_string.h
===================================================================
--- epan/value_string.h	(revision 19892)
+++ epan/value_string.h	(working copy)
@@ -34,6 +34,13 @@
   const gchar   *strptr;
 } value_string;
 
+/* Struct for the rval_to_str, match_strrval_idx, and match_strrval functions */
+typedef struct _range_string {
+  guint32        value_min;
+  guint32        value_max;
+  const gchar   *strptr;
+} range_string;
+
 /* #define VS_DEF(x) { x, #x } */
 /* #define VS_END    { 0, NULL } */
 
@@ -61,4 +68,21 @@
 extern const char *decode_enumerated_bitfield_shifted(guint32 val, guint32 mask,
   int width, const value_string *tab, const char *fmt);
 
+
+/* ranges aware versions */
+
+/* Tries to match val against each range in the range_string array rs.
+   Returns the associated string ptr on a match.
+   Formats val with fmt, and returns the resulting string, on failure. */
+extern const gchar* rval_to_str(guint32 val, const range_string *rs, const char *fmt);
+
+/* Tries to match val against each range in the range_string array rs.
+   Returns the associated string ptr, and sets "*idx" to the index in
+   that table, on a match, and returns NULL, and sets "*idx" to -1,
+   on failure. */
+extern const gchar *match_strrval_idx(guint32 val, const range_string *rs, gint *idx);
+
+/* Like match_strrval_idx(), but doesn't return the index. */
+extern const gchar *match_strrval(guint32 val, const range_string *rs);
+
 #endif /* __VALUE_STRING_H__ */
Index: epan/dissectors/packet-ospf.c
===================================================================
--- epan/dissectors/packet-ospf.c	(revision 19892)
+++ epan/dissectors/packet-ospf.c	(working copy)
@@ -202,6 +202,16 @@
     {0, NULL},
 };
 
+/* FF: from www.iana.org/assignments/bandwidth-constraints-model-ids */
+static const range_string mpls_link_stlv_bcmodel_rvals[] = {
+    { 0,     0, "(Russian Dolls Model - RDM)"                       },
+    { 1,     1, "(Maximum Allocation Model - MAM)"                  },
+    { 2,     2, "(Maximum Allocation with Reservation Model - MAR)" },
+    { 3,   239, "(Unassigned, Specification Required)"              },
+    { 240, 255, "(Reserved, Private Use)"                           },
+    { 0,     0, NULL                                                }
+};
+
 #define OSPF_V2_ROUTER_LSA_FLAG_B 0x01
 #define OSPF_V2_ROUTER_LSA_FLAG_E 0x02
 #define OSPF_V2_ROUTER_LSA_FLAG_V 0x04
@@ -585,7 +595,7 @@
        BASE_HEX, NULL, 0x0, "MPLS/TE Link Resource Class/Color", HFILL }},
     {&ospf_filter[OSPFF_LS_MPLS_BC_MODEL_ID],
      { "MPLS/DSTE Bandwidth Constraints Model Id", "ospf.mpls.bc", FT_UINT8,
-       BASE_HEX, NULL, 0x0, "MPLS/DSTE Bandwidth Constraints Model Id", HFILL }},
+       BASE_DEC, NULL, 0x0, "MPLS/DSTE Bandwidth Constraints Model Id", HFILL }},
 
     {&ospf_filter[OSPFF_V2_OPTIONS],
      { "Options", "ospf.v2.options", FT_UINT8, BASE_HEX,
@@ -1752,10 +1762,16 @@
 		    proto_tree_add_text(stlv_tree, tvb, stlv_offset+2, 2, "TLV Length: %u",
 					stlv_len);
 		    
-		    proto_tree_add_item(stlv_tree,
-                                        ospf_filter[OSPFF_LS_MPLS_BC_MODEL_ID],
-                                        tvb, stlv_offset+4, 1, FALSE);
-		    
+		    proto_tree_add_uint_format(stlv_tree,
+					       ospf_filter[OSPFF_LS_MPLS_BC_MODEL_ID],
+					       tvb, stlv_offset+4, 1, 
+					       tvb_get_guint8(tvb, stlv_offset+4),
+					       "MPLS/DSTE Bandwidth Constraints Model Id: %u %s",
+					       tvb_get_guint8(tvb, stlv_offset+4),
+					       rval_to_str(tvb_get_guint8(tvb, stlv_offset+4),
+							   mpls_link_stlv_bcmodel_rvals, 
+							   "Unknown"));
+
 		    /* 3 octets reserved +5, +6 and +7 (all 0x00) */
 		    if(tvb_memeql(tvb, stlv_offset+5, allzero, 3) == -1) {
 			proto_tree_add_text(stlv_tree, tvb, stlv_offset+5, 3, 

Index: epan/dissectors/packet-ospf.c
===================================================================
--- epan/dissectors/packet-ospf.c	(revision 19892)
+++ epan/dissectors/packet-ospf.c	(working copy)
@@ -202,6 +202,14 @@
     {0, NULL},
 };
 
+/* FF: from www.iana.org/assignments/bandwidth-constraints-model-ids */
+static const value_string mpls_link_stlv_bcmodel_vals[] = {
+    { 0, "Russian Dolls Model - RDM"                       },
+    { 1, "Maximum Allocation Model - MAM"                  },
+    { 2, "Maximum Allocation with Reservation Model - MAR" },
+    { 0, NULL                                              }
+};
+
 #define OSPF_V2_ROUTER_LSA_FLAG_B 0x01
 #define OSPF_V2_ROUTER_LSA_FLAG_E 0x02
 #define OSPF_V2_ROUTER_LSA_FLAG_V 0x04
@@ -585,7 +593,7 @@
        BASE_HEX, NULL, 0x0, "MPLS/TE Link Resource Class/Color", HFILL }},
     {&ospf_filter[OSPFF_LS_MPLS_BC_MODEL_ID],
      { "MPLS/DSTE Bandwidth Constraints Model Id", "ospf.mpls.bc", FT_UINT8,
-       BASE_HEX, NULL, 0x0, "MPLS/DSTE Bandwidth Constraints Model Id", HFILL }},
+       BASE_DEC, VALS(mpls_link_stlv_bcmodel_vals), 0x0, "MPLS/DSTE Bandwidth Constraints Model Id", HFILL }},
 
     {&ospf_filter[OSPFF_V2_OPTIONS],
      { "Options", "ospf.v2.options", FT_UINT8, BASE_HEX,
@@ -1752,9 +1760,23 @@
 		    proto_tree_add_text(stlv_tree, tvb, stlv_offset+2, 2, "TLV Length: %u",
 					stlv_len);
 		    
-		    proto_tree_add_item(stlv_tree,
-                                        ospf_filter[OSPFF_LS_MPLS_BC_MODEL_ID],
-                                        tvb, stlv_offset+4, 1, FALSE);
+		    {
+			guint8 bc_model_id = tvb_get_guint8(tvb, stlv_offset+4);
+			
+			if(bc_model_id <= 2) {
+			    proto_tree_add_item(stlv_tree,
+						ospf_filter[OSPFF_LS_MPLS_BC_MODEL_ID],
+						tvb, stlv_offset+4, 1, FALSE);
+			} else if((bc_model_id >= 3) && (bc_model_id <= 239)) {
+			    proto_tree_add_text(stlv_tree, tvb, stlv_offset+4, 1, 
+						"MPLS/DSTE Bandwidth Constraints Model Id: Unassigned (%u)", 
+						bc_model_id);
+			} else {
+			    proto_tree_add_text(stlv_tree, tvb, stlv_offset+4, 1, 
+						"MPLS/DSTE Bandwidth Constraints Model Id: Private (%u)", 
+						bc_model_id);
+			}
+		    }
 		    
 		    /* 3 octets reserved +5, +6 and +7 (all 0x00) */
 		    if(tvb_memeql(tvb, stlv_offset+5, allzero, 3) == -1) {