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, 53 insertions, 43 deletions

diff --git a/epan/dissectors/packet-fr.c b/epan/dissectors/packet-fr.c
index a44097f344..1f47356441 100644
--- a/epan/dissectors/packet-fr.c
+++ b/epan/dissectors/packet-fr.c
@@ -38,6 +38,7 @@
 
 #include <epan/packet.h>
 #include <epan/prefs.h>
+#include <epan/expert.h>
 #include <wiretap/wtap.h>
 
 #include "packet-llc.h"
@@ -110,6 +111,13 @@ static gint hf_fr_oui             = -1;
 static gint hf_fr_pid             = -1;
 static gint hf_fr_snaptype        = -1;
 static gint hf_fr_chdlctype       = -1;
+static gint hf_fr_first_addr_octet  = -1;
+static gint hf_fr_second_addr_octet  = -1;
+static gint hf_fr_third_addr_octet  = -1;
+
+static expert_field ei_fr_bogus_address = EI_INIT;
+static expert_field ei_fr_frame_relay_lapf = EI_INIT;
+static expert_field ei_fr_frame_relay_xid = EI_INIT;
 
 static dissector_handle_t eth_withfcs_handle;
 static dissector_handle_t gprs_ns_handle;
@@ -440,26 +448,24 @@ dissect_fr_common(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
        * should dissect it as such, if possible.
        */
       addr = 0;
-      if (tree) {
-        proto_tree_add_text(fr_tree, tvb, offset, 1,
+      proto_tree_add_expert_format(fr_tree, pinfo, &ei_fr_bogus_address, tvb, offset, 1,
                             "Bogus 1-octet address field");
-        offset++;
-      }
+      offset++;
     } else {
+        static const int *first_address_bits[] = {&hf_fr_upper_dlci, &hf_fr_cr, &hf_fr_ea, NULL};
+        static const int *second_address_bits[] = {&hf_fr_second_dlci, &hf_fr_fecn,
+                                        &hf_fr_becn, &hf_fr_de, &hf_fr_ea, NULL};
+        static const int *third_address_bits[] = {&hf_fr_third_dlci, &hf_fr_ea, NULL};
+
       /*
        * The first octet contains the upper 6 bits of the DLCI, as well
        * as the C/R bit.
        */
       addr = (fr_octet & FRELAY_UPPER_DLCI) >> 2;
       is_response = (fr_octet & FRELAY_CR);
-      if (tree) {
-        octet_item = proto_tree_add_text(fr_tree, tvb, offset, 1,
-                                         "First address octet: 0x%02x", fr_octet);
-        octet_tree = proto_item_add_subtree(octet_item, ett_fr_address);
-        proto_tree_add_uint(octet_tree, hf_fr_upper_dlci, tvb, offset, 1, fr_octet);
-        proto_tree_add_boolean(octet_tree, hf_fr_cr, tvb, offset, 1, fr_octet);
-        proto_tree_add_boolean(octet_tree, hf_fr_ea, tvb, offset, 1, fr_octet);
-      }
+
+      proto_tree_add_bitmask(fr_tree, tvb, offset, hf_fr_first_addr_octet,
+                                         ett_fr_address, first_address_bits, ENC_NA);
       offset++;
 
       /*
@@ -468,17 +474,8 @@ dissect_fr_common(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
        */
       fr_octet = tvb_get_guint8(tvb, offset);
       addr = (addr << 4) | ((fr_octet & FRELAY_SECOND_DLCI) >> 4);
-      if (tree) {
-        octet_item = proto_tree_add_text(fr_tree, tvb, offset, 1,
-                                         "Second address octet: 0x%02x",
-                                         fr_octet);
-        octet_tree = proto_item_add_subtree(octet_item, ett_fr_address);
-        proto_tree_add_uint(octet_tree, hf_fr_second_dlci, tvb, offset, 1, fr_octet);
-        proto_tree_add_boolean(octet_tree, hf_fr_fecn, tvb, 0, offset, fr_octet);
-        proto_tree_add_boolean(octet_tree, hf_fr_becn, tvb, 0, offset, fr_octet);
-        proto_tree_add_boolean(octet_tree, hf_fr_de, tvb, 0, offset, fr_octet);
-        proto_tree_add_boolean(octet_tree, hf_fr_ea, tvb, offset, 1, fr_octet);
-      }
+      proto_tree_add_bitmask(fr_tree, tvb, offset, hf_fr_second_addr_octet,
+                                         ett_fr_address, second_address_bits, ENC_NA);
       offset++;
 
       if (!(fr_octet & FRELAY_EA)) {
@@ -495,34 +492,24 @@ dissect_fr_common(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
            * 7 more bits of DLCI.
            */
           addr = (addr << 7) | ((fr_octet & FRELAY_THIRD_DLCI) >> 1);
-          if (tree) {
-            octet_item = proto_tree_add_text(fr_tree, tvb, offset, 1,
-                                             "Third address octet: 0x%02x",
-                                             fr_octet);
-            octet_tree = proto_item_add_subtree(octet_item, ett_fr_address);
-            proto_tree_add_uint(octet_tree, hf_fr_third_dlci, tvb, offset, 1, fr_octet);
-            proto_tree_add_boolean(octet_tree, hf_fr_ea, tvb, offset, 1, fr_octet);
-          }
+          proto_tree_add_bitmask(fr_tree, tvb, offset, hf_fr_third_addr_octet,
+                                         ett_fr_address, third_address_bits, ENC_NA);
           offset++;
           fr_octet = tvb_get_guint8(tvb, offset);
           while (!(fr_octet & FRELAY_EA)) {
             /*
              * Bogus!  More than 4 octets of address.
              */
-            if (tree) {
-              proto_tree_add_text(fr_tree, tvb, offset, 1,
+            proto_tree_add_expert_format(fr_tree, pinfo, &ei_fr_bogus_address, tvb, offset, 1,
                                  "Bogus extra address octet");
-            }
             offset++;
             fr_octet = tvb_get_guint8(tvb, offset);
           }
         }
-        if (tree) {
-          octet_item = proto_tree_add_text(fr_tree, tvb, offset, 1,
-                                           "Final address octet: 0x%02x",
+
+        octet_tree = proto_tree_add_subtree_format(fr_tree, tvb, offset, 1,
+                                           ett_fr_address, NULL, "Final address octet: 0x%02x",
                                            fr_octet);
-          octet_tree = proto_item_add_subtree(octet_item, ett_fr_address);
-        }
 
         /*
          * Last octet - contains lower DLCI or DL-CORE control, DLCI or
@@ -681,8 +668,7 @@ dissect_fr_nlpid(tvbuff_t *tvb, int offset, packet_info *pinfo,
   proto_item_set_end(ti, tvb, offset);
   fr_nlpid = tvb_get_guint8 (tvb,offset);
   if (fr_nlpid == 0) {
-    if (tree)
-      proto_tree_add_text(fr_tree, tvb, offset, 1, "Padding");
+    proto_tree_add_uint_format(fr_tree, hf_fr_nlpid, tvb, offset, 1, fr_nlpid, "Padding");
     offset++;
     if (ti != NULL) {
       /* Include the padding in the top-level protocol tree item. */
@@ -782,14 +768,14 @@ dissect_fr_nlpid(tvbuff_t *tvb, int offset, packet_info *pinfo,
 static void
 dissect_lapf(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-  proto_tree_add_text(tree, tvb, 0, 0, "Frame relay lapf not yet implemented");
+  proto_tree_add_expert(tree, pinfo, &ei_fr_frame_relay_lapf, tvb, 0, 0);
   call_dissector(data_handle,tvb_new_subset_remaining(tvb,0),pinfo,tree);
 }
 
 static void
 dissect_fr_xid(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-  proto_tree_add_text(tree, tvb, 0, 0, "Frame relay xid not yet implemented");
+  proto_tree_add_expert(tree, pinfo, &ei_fr_frame_relay_xid, tvb, 0, 0);
   call_dissector(data_handle,tvb_new_subset_remaining(tvb,0),pinfo,tree);
 }
 
@@ -948,6 +934,21 @@ proto_register_fr(void)
         FT_UINT16, BASE_HEX, VALS(chdlc_vals), 0x0,
         "Frame Relay Cisco HDLC Encapsulated Protocol", HFILL }},
 
+    { &hf_fr_first_addr_octet,
+      { "First address octet", "fr.first_addr_octet",
+        FT_UINT8, BASE_HEX, NULL, 0x0,
+        NULL, HFILL }},
+
+    { &hf_fr_second_addr_octet,
+      { "Second address octet", "fr.second_addr_octet",
+        FT_UINT8, BASE_HEX, NULL, 0x0,
+        NULL, HFILL }},
+
+    { &hf_fr_third_addr_octet,
+      { "Third address octet", "fr.third_addr_octet",
+        FT_UINT8, BASE_HEX, NULL, 0x0,
+        NULL, HFILL }},
+
   };
 
   /* Setup protocol subtree array */
@@ -956,6 +957,12 @@ proto_register_fr(void)
     &ett_fr_address,
     &ett_fr_control,
   };
+  static ei_register_info ei[] = {
+    { &ei_fr_bogus_address, { "fr.bogus_address", PI_PROTOCOL, PI_WARN, "Bogus address", EXPFILL }},
+    { &ei_fr_frame_relay_lapf, { "fr.frame_relay.lapf", PI_UNDECODED, PI_WARN, "Frame relay lapf not yet implemented", EXPFILL }},
+    { &ei_fr_frame_relay_xid, { "fr.frame_relay.xid", PI_UNDECODED, PI_WARN, "Frame relay xid not yet implemented", EXPFILL }},
+  };
+
   static const enum_val_t fr_encap_options[] = {
     { "frf-3.2", "FRF 3.2/Cisco HDLC", FRF_3_2 },
     { "gprs-ns", "GPRS Network Service", GPRS_NS },
@@ -963,10 +970,13 @@ proto_register_fr(void)
     { NULL, NULL, 0 },
   };
   module_t *frencap_module;
+  expert_module_t* expert_fr;
 
   proto_fr = proto_register_protocol("Frame Relay", "FR", "fr");
   proto_register_field_array(proto_fr, hf, array_length(hf));
   proto_register_subtree_array(ett, array_length(ett));
+  expert_fr = expert_register_protocol(proto_fr);
+  expert_register_field_array(expert_fr, ei, array_length(ei));
 
   fr_subdissector_table = register_dissector_table("fr.nlpid",
                                                    "Frame Relay NLPID", FT_UINT8, BASE_HEX);