Eliminate proto_tree_add_text from some dissectors.

Change-Id: I6f1710a093fc548c718defa9b40ab68877ede977
Reviewed-on: https://code.wireshark.org/review/3470
Reviewed-by: Michael Mann <mmann78@netscape.net>
Petri-Dish: Michael Mann <mmann78@netscape.net>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 61 insertions, 49 deletions

diff --git a/epan/dissectors/packet-gmrp.c b/epan/dissectors/packet-gmrp.c
index 9247682ac3..7d9d1aad3d 100644
--- a/epan/dissectors/packet-gmrp.c
+++ b/epan/dissectors/packet-gmrp.c
@@ -29,6 +29,7 @@
 #include <glib.h>
 
 #include <epan/packet.h>
+#include <epan/expert.h>
 #include <epan/llcsaps.h>
 
 void proto_register_gmrp(void);
@@ -42,13 +43,16 @@ static int hf_gmrp_attribute_length = -1;
 static int hf_gmrp_attribute_event = -1;
 static int hf_gmrp_attribute_value_group_membership = -1;
 static int hf_gmrp_attribute_value_service_requirement = -1;
-/*static int hf_gmrp_end_of_mark = -1;*/
+static int hf_gmrp_end_of_mark = -1;
 
 /* Initialize the subtree pointers */
 static gint ett_gmrp = -1;
-/*static gint ett_gmrp_message = -1;
+static gint ett_gmrp_message = -1;
 static gint ett_gmrp_attribute_list = -1;
-static gint ett_gmrp_attribute = -1;*/
+/*static gint ett_gmrp_attribute = -1;*/
+
+static expert_field ei_gmrp_proto_id = EI_INIT;
+
 
 static dissector_handle_t data_handle;
 
@@ -109,7 +113,7 @@ static void
 dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
     proto_item   *ti;
-    proto_tree   *gmrp_tree;
+    proto_tree   *gmrp_tree, *msg_tree, *attr_tree;
     guint16       protocol_id;
     guint8        octet;
     guint8        attribute_type;
@@ -119,8 +123,6 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 
     col_set_str(pinfo->cinfo, COL_INFO, "GMRP");
 
-    if (tree)
-    {
         ti = proto_tree_add_item(tree, proto_gmrp, tvb, 0, -1, ENC_NA);
 
         gmrp_tree = proto_item_add_subtree(ti, ett_gmrp);
@@ -128,11 +130,9 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
         /* Read in GARP protocol ID */
         protocol_id = tvb_get_ntohs(tvb, GARP_PROTOCOL_ID);
 
-        proto_tree_add_uint_format(gmrp_tree, hf_gmrp_proto_id, tvb,
-                                   GARP_PROTOCOL_ID, (int)sizeof(guint16),
-                                   protocol_id,
-                                   "Protocol Identifier: 0x%04x (%s)",
-                                   protocol_id,
+        ti = proto_tree_add_uint_format_value(gmrp_tree, hf_gmrp_proto_id, tvb,
+                                   GARP_PROTOCOL_ID, 2, protocol_id,
+                                   "0x%04x (%s)", protocol_id,
                                    protocol_id == GARP_DEFAULT_PROTOCOL_ID ?
                                      "GARP Multicast Registration Protocol" :
                                      "Unknown Protocol");
@@ -140,16 +140,15 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
         /* Currently only one protocol ID is supported */
         if (protocol_id != GARP_DEFAULT_PROTOCOL_ID)
         {
-            proto_tree_add_text(gmrp_tree, tvb, GARP_PROTOCOL_ID, (int)sizeof(guint16),
-                "   (Warning: this version of Wireshark only knows about protocol id = 1)");
+            expert_add_info(pinfo, ti, &ei_gmrp_proto_id);
             call_dissector(data_handle,
-                tvb_new_subset_remaining(tvb, GARP_PROTOCOL_ID + (int)sizeof(guint16)),
+                tvb_new_subset_remaining(tvb, GARP_PROTOCOL_ID + 2),
                 pinfo, tree);
             return;
         }
 
-        offset += (int)sizeof(guint16);
-        length -= (int)sizeof(guint16);
+        offset += 2;
+        length -= 2;
 
         msg_index = 0;
 
@@ -168,8 +167,7 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                 /* End of GARP PDU */
                 if (msg_index)
                 {
-                    proto_tree_add_text(gmrp_tree, tvb, offset, (int)sizeof(guint8),
-                                        "End of pdu");
+                    proto_tree_add_uint_format(gmrp_tree, hf_gmrp_end_of_mark, tvb, offset, 1, octet, "End of pdu");
                     break;
                 }
                 else
@@ -181,14 +179,14 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                 }
             }
 
-            offset += (int)sizeof(guint8);
-            length -= (int)sizeof(guint8);
+            offset += 1;
+            length -= 1;
 
-            msg_item = proto_tree_add_text(gmrp_tree, tvb, msg_start, -1,
-                                           "Message %d", msg_index + 1);
+            msg_tree = proto_tree_add_subtree_format(gmrp_tree, tvb, msg_start, -1,
+                                           ett_gmrp_message, &msg_item, "Message %d", msg_index + 1);
 
-            proto_tree_add_uint(gmrp_tree, hf_gmrp_attribute_type, tvb,
-                                msg_start, (int)sizeof(guint8), octet);
+            proto_tree_add_uint(msg_tree, hf_gmrp_attribute_type, tvb,
+                                msg_start, 1, octet);
 
             /* GMRP supports Group Membership and Service Requirement as attribute types */
             if ( (octet != GMRP_ATTRIBUTE_TYPE_GROUP_MEMBERSHIP) && (octet != GMRP_ATTRIBUTE_TYPE_SERVICE_REQUIREMENT) )
@@ -217,11 +215,11 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                      */
                     if (attr_index)
                     {
-                        proto_tree_add_text(gmrp_tree, tvb, offset,
-                                            (int)sizeof(guint8), "  End of mark");
+                        proto_tree_add_uint_format(msg_tree, hf_gmrp_end_of_mark, tvb, offset,
+                                            1, octet, "  End of mark");
 
-                        offset += (int)sizeof(guint8);
-                        length -= (int)sizeof(guint8);
+                        offset += 1;
+                        length -= 1;
 
                         proto_item_set_len(msg_item, offset - msg_start);
                         break;
@@ -238,23 +236,23 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                 {
                     guint8   event;
 
-                    offset += (int)sizeof(guint8);
-                    length -= (int)sizeof(guint8);
+                    offset += 1;
+                    length -= 1;
 
-                    attr_item = proto_tree_add_text(gmrp_tree, tvb,
-                                                    attr_start, -1, "  Attribute %d", attr_index + 1);
+                    attr_tree = proto_tree_add_subtree_format(msg_tree, tvb, attr_start, -1,
+                                                    ett_gmrp_attribute_list, &attr_item, "  Attribute %d", attr_index + 1);
 
-                    proto_tree_add_uint(gmrp_tree, hf_gmrp_attribute_length,
-                                        tvb, attr_start, (int)sizeof(guint8), octet);
+                    proto_tree_add_uint(attr_tree, hf_gmrp_attribute_length,
+                                        tvb, attr_start, 1, octet);
 
                     /* Read in attribute event */
                     event = tvb_get_guint8(tvb, offset);
 
-                    proto_tree_add_uint(gmrp_tree, hf_gmrp_attribute_event,
-                                        tvb, offset, (int)sizeof(guint8), event);
+                    proto_tree_add_uint(attr_tree, hf_gmrp_attribute_event,
+                                        tvb, offset, 1, event);
 
-                    offset += (int)sizeof(guint8);
-                    length -= (int)sizeof(guint8);
+                    offset += 1;
+                    length -= 1;
 
                     switch (event) {
 
@@ -287,21 +285,21 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                         if ( GMRP_ATTRIBUTE_TYPE_GROUP_MEMBERSHIP == attribute_type )
                         {
                             /* Group Membership */
-                            proto_tree_add_item(gmrp_tree, hf_gmrp_attribute_value_group_membership,
-                                                tvb, offset, (int)(6*sizeof(guint8)), ENC_NA);
+                            proto_tree_add_item(attr_tree, hf_gmrp_attribute_value_group_membership,
+                                                tvb, offset, 6, ENC_NA);
 
-                            offset += (int)(6*sizeof(guint8));
-                            length -= (int)(6*sizeof(guint8));
+                            offset += 6;
+                            length -= 6;
                         }
                         else
                             if ( GMRP_ATTRIBUTE_TYPE_SERVICE_REQUIREMENT == attribute_type )
                             {
                                 /* Service Requirement */
-                                proto_tree_add_item(gmrp_tree, hf_gmrp_attribute_value_service_requirement,
-                                                    tvb, offset, (int)sizeof(guint8), ENC_BIG_ENDIAN);
+                                proto_tree_add_item(attr_tree, hf_gmrp_attribute_value_service_requirement,
+                                                    tvb, offset, 1, ENC_BIG_ENDIAN);
 
-                                offset += (int)sizeof(guint8);
-                                length -= (int)sizeof(guint8);
+                                offset += 1;
+                                length -= 1;
                             }
                             else
                             {
@@ -328,7 +326,6 @@ dissect_gmrp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 
             msg_index++;
         }
-    }
 }
 
 
@@ -339,7 +336,7 @@ proto_register_gmrp(void)
 {
     static hf_register_info hf[] = {
         { &hf_gmrp_proto_id,
-          { "Protocol ID", "gmrp.protocol_id",
+          { "Protocol Identifier", "gmrp.protocol_id",
             FT_UINT16,      BASE_HEX,      NULL,  0x0,
             NULL , HFILL }
         },
@@ -367,14 +364,27 @@ proto_register_gmrp(void)
           { "Value",       "gmrp.attribute_value_service_requirement",
             FT_UINT8,        BASE_HEX,      NULL,  0x0,
             NULL , HFILL }
+        },
+        { &hf_gmrp_end_of_mark,
+          { "End of mark",       "gmrp.end_of_mark",
+            FT_UINT8,        BASE_HEX,      NULL,  0x0,
+            NULL , HFILL }
         }
 
     };
 
     static gint *ett[] = {
-        &ett_gmrp
+        &ett_gmrp,
+        &ett_gmrp_message,
+        &ett_gmrp_attribute_list
+    };
+
+    static ei_register_info ei[] = {
+        { &ei_gmrp_proto_id, { "gmrp.protocol_id.not_gmrp", PI_UNDECODED, PI_WARN, "This version of Wireshark only knows about protocol id = 1", EXPFILL }},
     };
 
+    expert_module_t* expert_gmrp;
+
     /* Register the protocol name and description for GMRP */
     proto_gmrp = proto_register_protocol("GARP Multicast Registration Protocol", "GMRP", "gmrp");
 
@@ -382,6 +392,8 @@ proto_register_gmrp(void)
      * used by GMRP */
     proto_register_field_array(proto_gmrp, hf, array_length(hf));
     proto_register_subtree_array(ett, array_length(ett));
+    expert_gmrp = expert_register_protocol(proto_gmrp);
+    expert_register_field_array(expert_gmrp, ei, array_length(ei));
 
     register_dissector("gmrp", dissect_gmrp, proto_gmrp);