More rework of the code:

 - Refactor to use a separate function for common code;
 - Prefs callback not required;
 - Use val_to_str(...);
 - Fix issues relating to incorrect bytes highlighted in the hex view;


git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@33271 f5534014-38df-0310-8fa8-9805f1628bb7

1 files changed, 86 insertions, 138 deletions

diff --git a/epan/dissectors/packet-gmhdr.c b/epan/dissectors/packet-gmhdr.c
index 36512218ef..b7ceaff28f 100644
--- a/epan/dissectors/packet-gmhdr.c
+++ b/epan/dissectors/packet-gmhdr.c
@@ -39,6 +39,12 @@
 #define GMHDR_FTYPE_TIMESTAMP_NTP       4
 #define GMHDR_FTYPE_TIMESTAMP_EXT       5
 
+static const value_string gmhdr_ftype_timestamp[] = {
+  { GMHDR_FTYPE_TIMESTAMP_LOCAL, "Local" },
+  { GMHDR_FTYPE_TIMESTAMP_NTP,   "NTP" },
+  { GMHDR_FTYPE_TIMESTAMP_EXT,   "External" }
+};
+
 #define GMHDR_SRCPORT_PLFM_MASK         0x00f80000
 #define GMHDR_SRCPORT_GID_MASK          0x00078000
 #define GMHDR_SRCPORT_BID_MASK          0x00007c00
@@ -54,8 +60,6 @@ static const value_string gmhdr_plfm_str[] = {
   { 0, NULL }
 };
 
-void proto_reg_handoff_gmhdr(void);
-
 static gboolean gmhdr_summary_in_tree = TRUE;
 
 static int proto_gmhdr = -1;
@@ -74,83 +78,83 @@ static int hf_gmhdr_trailer = -1;
 static gint ett_gmhdr = -1;
 static gint ett_srcport = -1;
 
+
+
+static void
+dissect_gmtlv(tvbuff_t *tvb, packet_info *pinfo, proto_tree *gmhdr_tree, guint offset, guint16 length)
+{
+  proto_tree *ti;
+  proto_tree *srcport_tree;
+  guint16     fl;
+
+  while (length > 1) {
+    guint16 tl = tvb_get_ntohs(tvb, offset);
+    offset += 2; /* type + len */
+    length -= 2;
+
+    fl = tl & 0xff;
+    switch (tl >> 8) {
+      case GMHDR_FTYPE_SRCPORT: {
+        guint16 pid;
+        guint32 tv = tvb_get_ntohl(tvb, offset) >> 8; /* Only 24-bit field */
+        ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_srcport,      tvb, offset, fl, FALSE);
+        srcport_tree = proto_item_add_subtree(ti, ett_srcport);
+        proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_plfm, tvb, offset, fl, FALSE);         
+        proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_gid,  tvb, offset, fl, FALSE);         
+        proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_bid,  tvb, offset, fl, FALSE);         
+        ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_pid,  tvb, offset, fl, FALSE);         
+        /* If not GV-2404, we need different formula here */
+        pid = ((tv & GMHDR_SRCPORT_PID_MASK) >> GMHDR_SRCPORT_PID_SHFT) - 24;
+        if (pid >= 1 && pid <= 4) {
+          proto_item_append_text(ti, " (g%d)", pid);
+        }
+        break;
+      }
+      case GMHDR_FTYPE_PKTSIZE:
+        proto_tree_add_item(gmhdr_tree, hf_gmhdr_pktsize, tvb, offset, fl, FALSE);
+        break;
+      case GMHDR_FTYPE_TIMESTAMP_LOCAL: 
+      case GMHDR_FTYPE_TIMESTAMP_NTP:
+      case GMHDR_FTYPE_TIMESTAMP_EXT:
+        ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_timestamp, tvb, offset, fl, FALSE);
+        proto_item_append_text(ti, "; Source: %s", val_to_str(tl>>8, gmhdr_ftype_timestamp, "Unknown")); 
+        break;
+      default:
+        ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_generic, tvb, offset, fl, FALSE);
+        proto_item_append_text(ti, " [Id: %u, Length: %u]", tl >> 8, fl);
+        break;
+    }
+    /* Adjust for the field length */
+    offset += fl;
+    length -= fl;
+  }
+}
+
+
+
 static void
 dissect_gmhdr(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
   proto_tree *ti;
   gint16 length;
-  guint16 fl;
   volatile guint16 encap_proto;
   volatile gboolean is_802_2;
-  proto_tree *volatile gmhdr_tree;
-  proto_tree *srcport_tree;
-  unsigned offset = 0;
+  proto_tree *volatile gmhdr_tree = NULL;
+  guint offset = 0;
 
-  length = tvb_get_guint8(tvb, offset); /* This is length of Gigamon header */
-
-  gmhdr_tree = NULL;
+  length = tvb_get_guint8(tvb, offset); /* Length of the Gigamon header */
 
   if (tree) {
-    ti = proto_tree_add_item(tree, proto_gmhdr, tvb, offset, length + 2, FALSE);
+    ti = proto_tree_add_item(tree, proto_gmhdr, tvb, offset, length, FALSE);
 
     if (gmhdr_summary_in_tree) {
-        proto_item_append_text(ti, ", Length: %u", length);
+      proto_item_append_text(ti, ", Length: %u", length);
     }
 
     gmhdr_tree = proto_item_add_subtree(ti, ett_gmhdr);
+    dissect_gmtlv(tvb, pinfo, gmhdr_tree, offset+1, length-1);
 
-    /* Adjust one byte for length */
-    offset += 1;
-    length -= 1;
-    while (length > 1) {
-      guint16 tl = tvb_get_ntohs(tvb, offset);
-      offset += 2; /* type + len */
-      length -= 2;
-      fl = tl & 0xff;
-      switch (tl >> 8) {
-        case GMHDR_FTYPE_SRCPORT: {
-          guint16 pid;
-          guint32 tv = tvb_get_ntohl(tvb, offset) >> 8; /* Only 24-bit field */
-          ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_srcport,      tvb, offset, fl, FALSE);
-	  srcport_tree = proto_item_add_subtree(ti, ett_srcport);
-          ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_plfm, tvb, offset, fl, FALSE);         
-          ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_gid,  tvb, offset, fl, FALSE);         
-          ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_bid,  tvb, offset, fl, FALSE);         
-          ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_pid,  tvb, offset, fl, FALSE);         
-          /* If not GV-2404, we need different formula here */
-          pid = ((tv & GMHDR_SRCPORT_PID_MASK) >> GMHDR_SRCPORT_PID_SHFT) - 24;
-          if (pid >= 1 && pid <= 4) {
-            proto_item_append_text(ti, " (g%d)", pid);
-          }
-          break;
-        }
-        case GMHDR_FTYPE_PKTSIZE:
-          proto_tree_add_item(gmhdr_tree, hf_gmhdr_pktsize, tvb, offset, fl, FALSE);
-          break;
-        case GMHDR_FTYPE_TIMESTAMP_LOCAL: 
-        case GMHDR_FTYPE_TIMESTAMP_NTP:
-        case GMHDR_FTYPE_TIMESTAMP_EXT: {
-          char *tssrc = "Unknown";
-          ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_timestamp, tvb, offset, fl, FALSE);
-        
-          switch (tl >> 8) {
-            case GMHDR_FTYPE_TIMESTAMP_LOCAL: tssrc = "Local";    break;
-            case GMHDR_FTYPE_TIMESTAMP_NTP:   tssrc = "NTP";      break;
-            case GMHDR_FTYPE_TIMESTAMP_EXT:   tssrc = "External"; break;
-          }
-          proto_item_append_text(ti, ", Source: %s", tssrc);
-          break;
-        }
-        default:
-          ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_generic, tvb, offset, fl, FALSE);
-          proto_item_append_text(ti, " [Id: %u, Length: %u]", tl >> 8, fl);
-          break;
-      }
-      /* Adjust for the field length */
-      offset += fl;
-      length -= fl;
-    }
-  }
+  } /* if (tree) */
 
   offset += length;
   encap_proto = tvb_get_ntohs(tvb, offset);
@@ -187,37 +191,35 @@ dissect_gmtrailer(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 {
   proto_tree *ti;
   gint16 length;
-  guint16 fl;
-  proto_tree *volatile gmhdr_tree;
-  proto_tree *srcport_tree;
-  unsigned offset = 0;
+  proto_tree *gmhdr_tree = NULL;
+  guint offset;
   guint16 cksum, comp_cksum;
   
   /* See if this packet has a Gigamon trailer, if yes, then decode it */ 
-  if (tvb_get_ntohs(tvb, tvb->length - 4) != ETHERTYPE_GIGAMON) {
+  if (tvb_get_ntohs(tvb, -4) != ETHERTYPE_GIGAMON) {
     return 0;
   }
 
-  offset = tvb->length - 2; /* 2 byte checksum */
-  cksum = tvb_get_ntohs(tvb, offset);
-  offset -= 3; /* 1 byte length + 2 byte ET */
-  length = tvb_get_guint8(tvb, offset); /* This is length of Gigamon header */
+  offset  = tvb_reported_length(tvb) - 2; /* 2 byte checksum */
+  cksum   = tvb_get_ntohs(tvb, offset);
+  offset -= 3;  /* 1 byte length + 2 byte ET */
+  length  = tvb_get_guint8(tvb, offset); /* length of Gigamon header */
+  offset -= length;
 
-  /* Verify the checksum, if not valid, it means that the trailer is not valid */
+  /* Verify the checksum; if not valid, it means that the trailer is not valid */
   {
     vec_t vec;
     vec.len = length + 3;
-    vec.ptr = tvb_get_ptr(tvb, offset - length, vec.len);
+    vec.ptr = tvb_get_ptr(tvb, offset, vec.len);
 
     comp_cksum = in_cksum(&vec, 1);
     if (pntohs(&comp_cksum) != cksum) {
       return 0;
     }
   }
-  gmhdr_tree = NULL;
 
   if (tree) {
-    ti = proto_tree_add_item(tree, proto_gmhdr, tvb, offset - length, length + 5, FALSE);
+    ti = proto_tree_add_item(tree, proto_gmhdr, tvb, offset, length + 5, FALSE);
 
     if (gmhdr_summary_in_tree) {
         proto_item_append_text(ti, ", Length: %u, Checksum: 0x%x", length, cksum);
@@ -225,59 +227,9 @@ dissect_gmtrailer(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree)
 
     gmhdr_tree = proto_item_add_subtree(ti, ett_gmhdr);
 
-    /* Adjust for length */
-    offset -= length;
-    while (length > 1) {
-      guint16 tl = tvb_get_ntohs(tvb, offset);
-      offset += 2; /* type + len */
-      length -= 2;
-      fl = tl & 0xff;
-      switch (tl >> 8) {
-        case GMHDR_FTYPE_SRCPORT: {
-          guint16 pid;
-          guint32 tv = tvb_get_ntohl(tvb, offset) >> 8; /* Only 24-bit field */
-          ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_srcport,      tvb, offset, fl, FALSE);
-	  srcport_tree = proto_item_add_subtree(ti, ett_srcport);
-          ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_plfm, tvb, offset, fl, FALSE);         
-          ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_gid,  tvb, offset, fl, FALSE);         
-          ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_bid,  tvb, offset, fl, FALSE);         
-          ti = proto_tree_add_item(srcport_tree, hf_gmhdr_srcport_pid,  tvb, offset, fl, FALSE);         
-          /* If not GV-2404, we need different formula here */
-          pid = ((tv & GMHDR_SRCPORT_PID_MASK) >> GMHDR_SRCPORT_PID_SHFT) - 24;
-          if (pid >= 1 && pid <= 4) {
-            proto_item_append_text(ti, " (g%d)", pid);
-          }
-          break;
-        }
-        case GMHDR_FTYPE_PKTSIZE:
-          proto_tree_add_item(gmhdr_tree, hf_gmhdr_pktsize, tvb, offset, fl, FALSE);
-          break;
-        case GMHDR_FTYPE_TIMESTAMP_LOCAL: 
-        case GMHDR_FTYPE_TIMESTAMP_NTP:
-        case GMHDR_FTYPE_TIMESTAMP_EXT: {
-          char *tssrc = "Unknown";
-          ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_timestamp, tvb, offset, fl, FALSE);
-
-          switch (tl >> 8) {
-            case GMHDR_FTYPE_TIMESTAMP_LOCAL: tssrc = "Local";    break;
-            case GMHDR_FTYPE_TIMESTAMP_NTP:   tssrc = "NTP";      break;
-            case GMHDR_FTYPE_TIMESTAMP_EXT:   tssrc = "External"; break;
-          }
-          proto_item_append_text(ti, ", Source: %s", tssrc);
-          break;
-        }
-        default:
-          ti = proto_tree_add_item(gmhdr_tree, hf_gmhdr_generic, tvb, offset, fl, FALSE);
-          proto_item_append_text(ti, " [Id: %u, Length: %u]", tl >> 8, fl);
-          break;
-      }
-      /* Adjust for the field length */
-      offset += fl;
-      length -= fl;
-    }
+    dissect_gmtlv(tvb, pinfo, gmhdr_tree, offset, length);
   }
-
-  return offset;
+  return tvb_reported_length(tvb);
 }
 
 void
@@ -328,7 +280,7 @@ proto_register_gmhdr(void)
   proto_register_field_array(proto_gmhdr, hf, array_length(hf));
   proto_register_subtree_array(ett, array_length(ett));
 
-  gmhdr_module = prefs_register_protocol(proto_gmhdr, proto_reg_handoff_gmhdr);
+  gmhdr_module = prefs_register_protocol(proto_gmhdr, NULL);
   prefs_register_bool_preference(gmhdr_module, "summary_in_tree",
         "Show Gigamon header summary in protocol tree",
         "Whether the Gigamon header summary line should be shown in the protocol tree",
@@ -338,14 +290,10 @@ proto_register_gmhdr(void)
 void
 proto_reg_handoff_gmhdr(void)
 {
-  static gboolean prefs_initialized = FALSE;
-  static dissector_handle_t gmhdr_handle;
-
-  if (!prefs_initialized) {
-    gmhdr_handle = create_dissector_handle(dissect_gmhdr, proto_gmhdr);
-    dissector_add("ethertype", ETHERTYPE_GIGAMON, gmhdr_handle);
-    heur_dissector_add("eth.trailer", dissect_gmtrailer, proto_gmhdr);
-    prefs_initialized = TRUE;
-  }
+  dissector_handle_t gmhdr_handle;
+
+  gmhdr_handle = create_dissector_handle(dissect_gmhdr, proto_gmhdr);
+  dissector_add("ethertype", ETHERTYPE_GIGAMON, gmhdr_handle);
+  heur_dissector_add("eth.trailer", dissect_gmtrailer, proto_gmhdr);
 }