[Michael Duigou <ethereal@xxxxxxxxxx>]

Some improvements for cleaner dissection of jxta messages.

Index: epan/dissectors/packet-jxta.c
===================================================================
--- epan/dissectors/packet-jxta.c	(revision 13594)
+++ epan/dissectors/packet-jxta.c	(working copy)
@@ -43,12 +43,17 @@
 #include <epan/prefs.h>
 #include "packet-tcp.h"
 
+static const char JXTA_UDP_SIG[] = { 'J', 'X', 'T', 'A' };
+static const char JXTA_MSG_SIG[] = { 'j', 'x', 'm', 'g' };
+static const char JXTA_MSGELEM_SIG[] = { 'j', 'x', 'e', 'l' };
+
 static int proto_jxta = -1;
 
 static int hf_jxta_udp = -1;
 static int hf_jxta_udpsig = -1;
 static int hf_jxta_welcome = -1;
 static int hf_jxta_framing = -1;
+static int hf_jxta_framing_header = -1;
 static int hf_jxta_framing_header_name_length = -1;
 static int hf_jxta_framing_header_name = -1;
 static int hf_jxta_framing_header_value_length = -1;
@@ -77,6 +82,7 @@
 static gint ett_jxta_welcome = -1;
 static gint ett_jxta_udp = -1;
 static gint ett_jxta_framing = -1;
+static gint ett_jxta_framing_header = -1;
 static gint ett_jxta_msg = -1;
 static gint ett_jxta_elem = -1;
 
@@ -100,6 +106,10 @@
   },
   { &hf_jxta_framing,
     { "JXTA Message Framing", "jxta.framing", FT_NONE, BASE_NONE, NULL, 0x0,
+      "JXTA Message Framing Headers", HFILL }
+  },
+  { &hf_jxta_framing_header,
+    { "Header", "jxta.framing.header", FT_NONE, BASE_NONE, NULL, 0x0,
       "JXTA Message Framing Header", HFILL }
   },
   { &hf_jxta_framing_header_name_length,
@@ -204,6 +214,7 @@
   &ett_jxta_welcome,
   &ett_jxta_udp,
   &ett_jxta_framing,
+  &ett_jxta_framing_header,
   &ett_jxta_msg,
   &ett_jxta_elem
 };
@@ -227,7 +238,7 @@
     Dissect a tvbuff containing a JXTA UDP header, JXTA Message framing and a JXTA Message
 **/
 static void dissect_jxta_udp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) {
-  proto_tree      *jxta_tree = NULL;
+  proto_tree      *jxta_udp_tree = NULL;
   proto_item      *ti;
 
   if (check_col(pinfo->cinfo, COL_PROTOCOL)) {
@@ -246,15 +257,15 @@
 
   if (tree) {
     ti = proto_tree_add_item(tree, hf_jxta_udp, tvb, 0, -1, FALSE);
-    jxta_tree = proto_item_add_subtree(ti, ett_jxta_udp);
+    jxta_udp_tree = proto_item_add_subtree(ti, ett_jxta_udp);
     
-    ti = proto_tree_add_item( jxta_tree, hf_jxta_udpsig, tvb, 0, 4, FALSE );
+    ti = proto_tree_add_item( jxta_udp_tree, hf_jxta_udpsig, tvb, 0, 4, FALSE );
   }
   
-  if( tvb_memeql(tvb, 0, "JXTA", 4) == 0 ) {
-    tvbuff_t* jxta_framed_message_tvb = tvb_new_subset( tvb, 4, -1, -1 );
+  if( tvb_memeql(tvb, 0, JXTA_UDP_SIG, sizeof(JXTA_UDP_SIG)) == 0 ) {
+    tvbuff_t* jxta_framed_message_tvb = tvb_new_subset( tvb, sizeof(JXTA_UDP_SIG), -1, -1 );
 
-    dissect_jxta_framing( jxta_framed_message_tvb, pinfo, tree );
+    dissect_jxta_framing( jxta_framed_message_tvb, pinfo, jxta_udp_tree );
   }
 }
 
@@ -277,29 +288,40 @@
     Dissect a tvbuff containing a JXTA Message framing and a JXTA Message
 **/
 static void dissect_jxta_framing(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) {
-  proto_tree      *jxta_tree = NULL;
-  proto_item      *ti;
+  proto_item      *framing_tree_item = NULL;
+  proto_tree      *framing_tree = NULL;
   guint offset = 0;
   tvbuff_t* jxta_message_tvb;
   
   if (tree) {
-    ti = proto_tree_add_item(tree, hf_jxta_framing, tvb, 0, -1, FALSE);
-    jxta_tree = proto_item_add_subtree(ti, ett_jxta_framing);
+    framing_tree_item = proto_tree_add_item(tree, hf_jxta_framing, tvb, 0, -1, FALSE);
+    framing_tree = proto_item_add_subtree(framing_tree_item, ett_jxta_framing);
     }
     
   /* parse framing headers */
   do {
     guint8 headernamelen = tvb_get_guint8( tvb, offset );
+    proto_item      *framing_header_tree_item = NULL;
+    proto_tree      *framing_header_tree = NULL;
 
     if(tree) {
-      proto_tree_add_item( jxta_tree, hf_jxta_framing_header_name_length, tvb, offset, 1, headernamelen );
+      framing_header_tree_item = proto_tree_add_item(framing_tree, hf_jxta_framing_header, tvb, 0, -1, FALSE);
+      framing_header_tree = proto_item_add_subtree(framing_header_tree_item, ett_jxta_framing_header);
+      
+      proto_tree_add_item( framing_header_tree, hf_jxta_framing_header_name_length, tvb, offset, 1, headernamelen );
     }
     
     if( tree && (headernamelen != 0) ) {
       /*
        * Put header name into protocol tree.
        */
-      proto_tree_add_item(jxta_tree, hf_jxta_framing_header_name, tvb, offset+1, headernamelen, FALSE);
+      guint8* headername = tvb_memdup( tvb, offset + 1, headernamelen );
+
+      proto_item_append_text(framing_header_tree_item, " \"%*.*s\"", headernamelen, headernamelen, headername );
+      
+      proto_tree_add_item(framing_header_tree, hf_jxta_framing_header_name, tvb, offset+1, headernamelen, FALSE);
+      
+      free(headername);
     }
     
     offset += 1 + headernamelen;
@@ -308,24 +330,32 @@
       guint16 headervaluelen = tvb_get_ntohs( tvb, offset );
 
       if( tree ) {
-        proto_tree_add_uint(jxta_tree, hf_jxta_framing_header_value_length, tvb, offset, 2, headervaluelen );
+        proto_tree_add_uint(framing_header_tree, hf_jxta_framing_header_value_length, tvb, offset, 2, headervaluelen );
 
         /** TODO bondolo Add specific handling for known header types */
 
         /*
          * Put header value into protocol tree.
          */
-        proto_tree_add_item(jxta_tree, hf_jxta_framing_header_value, tvb, offset+2, headervaluelen, FALSE );
+        proto_tree_add_item(framing_header_tree, hf_jxta_framing_header_value, tvb, offset+2, headervaluelen, FALSE );
       }
       
       offset += 2 + headervaluelen;
     }
     
+    if( tree ) {
+        proto_item_set_end( framing_header_tree_item, tvb, offset );
+    }
+
     if( 0 == headernamelen ) {
       break;
     }
   } while( TRUE );
   
+  if( tree ) {
+       proto_item_set_end( framing_tree_item, tvb, offset );
+  }
+
   jxta_message_tvb = tvb_new_subset( tvb, offset, -1, -1 );
 
   /* Call it a new layer and pass the tree as we got it */
@@ -338,6 +368,7 @@
 static void dissect_jxta_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) {
   proto_tree      *jxta_tree = NULL;
   proto_item      *ti;
+  unsigned int offset = 0;
 
   if (check_col(pinfo->cinfo, COL_PROTOCOL)) {
     col_set_str(pinfo->cinfo, COL_PROTOCOL, "JXTA");
@@ -355,24 +386,29 @@
 
   if (tree) {
     ti = proto_tree_add_item(tree, hf_jxta_message, tvb, 0, -1, FALSE);
-    jxta_tree = proto_item_add_subtree(ti, ett_jxta_udp);
+    jxta_tree = proto_item_add_subtree(ti, ett_jxta_msg);
   }
 
   if( tree ) {
-    proto_tree_add_item( jxta_tree, hf_jxta_message_sig, tvb, 0, 4, FALSE);
+    proto_tree_add_item( jxta_tree, hf_jxta_message_sig, tvb, 0, sizeof(JXTA_MSG_SIG), FALSE);
     
-    if( tvb_memeql(tvb, 0, "jxmg", 4) == 0) {
+    if( tvb_memeql(tvb, offset, JXTA_MSG_SIG, sizeof(JXTA_MSG_SIG)) == 0) {
         guint8 messageVersion;
         
-        messageVersion = tvb_get_guint8( tvb, sizeof(guint32) );
-        proto_tree_add_uint( jxta_tree, hf_jxta_message_version, tvb, sizeof(guint32), 1, messageVersion );
+        offset += sizeof(JXTA_MSG_SIG);
+
+        messageVersion = tvb_get_guint8( tvb, offset );
+        proto_tree_add_uint( jxta_tree, hf_jxta_message_version, tvb, offset, sizeof(guint8), messageVersion );
+        offset += sizeof(guint8);
         
         if( 0 == messageVersion ) {
             int eachNamespace;
             guint16 numberOfElements;
-            unsigned int offset = 7;
-            guint16 messageNamespaceCount = tvb_get_ntohs( tvb, 5 );
+            guint16 messageNamespaceCount = tvb_get_ntohs( tvb, offset );
+            offset += sizeof(guint16);
             
+            proto_tree_add_uint( jxta_tree, hf_jxta_message_namespaces_count, tvb, offset, sizeof(guint16), messageNamespaceCount );
+
             /* parse namespaces */
             /* TODO 20050103 bondolo Should record the namespaces and number them. */
             for( eachNamespace = 0; eachNamespace < messageNamespaceCount; eachNamespace++ ) {
@@ -391,23 +427,24 @@
             offset += sizeof(guint16);
             
             while( offset < tvb_reported_length(tvb) ) {
-                    proto_tree   *jxta_elem_tree = NULL;
-                    proto_item      *elem_ti;
+                proto_tree   *jxta_elem_tree = NULL;
+                proto_item      *elem_ti;
                     
-                    elem_ti = proto_tree_add_item(jxta_tree, hf_jxta_element, tvb, 0, -1, FALSE);
-                    jxta_elem_tree = proto_item_add_subtree(elem_ti, ett_jxta_elem);
+                elem_ti = proto_tree_add_item(jxta_tree, hf_jxta_element, tvb, offset, -1, FALSE);
+                jxta_elem_tree = proto_item_add_subtree(elem_ti, ett_jxta_elem);
 
                 /* gross hack for parsing of signature element */
                 element_parse :
                 {
-                proto_tree_add_item( jxta_tree, hf_jxta_element_sig, tvb, offset, 4, FALSE );
-                offset += 4;
-                if( tvb_memeql(tvb, offset - 4, "jxel", 4) == 0 ) {
+                proto_tree_add_item( jxta_elem_tree, hf_jxta_element_sig, tvb, offset, sizeof(JXTA_MSGELEM_SIG), FALSE );
+                if( tvb_memeql(tvb, offset, JXTA_MSGELEM_SIG, sizeof(JXTA_MSGELEM_SIG)) == 0 ) {
                     guint8 namespaceID;
                     guint8 flags;
                     guint16 nameLen;
                     guint32 elemContentLength;
 
+                    offset += sizeof(JXTA_MSGELEM_SIG);
+
                     namespaceID = tvb_get_guint8( tvb, offset );
                     proto_tree_add_uint( jxta_elem_tree, hf_jxta_element_namespaceid, tvb, offset, sizeof(guint8), namespaceID );
                     offset += sizeof(guint8);
@@ -460,7 +497,7 @@
                     }
                 }
                     
-                proto_item_set_end( elem_ti, tvb, offset - 1 );
+                proto_item_set_end( elem_ti, tvb, offset );
                 }
             }
         }