dpaux: Minor improvements

1. Pass dissector data to dpaux dissector directly instead of using p_get_proto_data.
2. Don't assume dissector data will always be present and default to "sink" if
that is the case.
3. tvb_memdup isn't needed for proto_tree_add_bytes
4. Use value_string to save switch cases.
5. Bugfix major/minor version dissection.

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

3 files changed, 41 insertions, 66 deletions

diff --git a/epan/dissectors/packet-dpaux.c b/epan/dissectors/packet-dpaux.c
index 232727e279..f79b89ff0e 100644
--- a/epan/dissectors/packet-dpaux.c
+++ b/epan/dissectors/packet-dpaux.c
@@ -17,13 +17,9 @@
 
 #include "packet-dpaux.h"
 
-/* Prototypes */
-/* (Required to prevent [-Wmissing-prototypes] warnings */
-void proto_reg_handoff_dpaux(void);
 void proto_register_dpaux(void);
 
-/* Initialize the protocol and registered fields */
-int proto_dpaux = -1;
+static int proto_dpaux = -1;
 
 static int hf_dpaux_transaction_type = -1;
 static int hf_dpaux_native_req_cmd = -1;
@@ -40,8 +36,8 @@ static int hf_00000 = -1;
 static int hf_00000_MINOR = -1;
 static int hf_00000_MAJOR = -1;
 static const int *reg00000_fields[] = {
-    &hf_00000_MINOR,
     &hf_00000_MAJOR,
+    &hf_00000_MINOR,
     NULL
 };
 
@@ -139,14 +135,12 @@ struct dpaux_register registers[] = {
 };
 
 static int
-dissect_dpaux_register(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
+dissect_dpaux_register(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree,
     unsigned int offset, unsigned int register_addr)
 {
     unsigned int k;
     struct dpaux_register *reg = NULL;
 
-    if (!pinfo) { }
-
     for (k = 0; k < G_N_ELEMENTS(registers); ++k) {
         if (registers[k].addr == register_addr) {
             reg = &registers[k];
@@ -212,8 +206,7 @@ dissect_dpaux_from_source(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 
 
     if (!is_read)
-        proto_tree_add_bytes(tree, hf_dpaux_data, tvb, 4, len,
-            (guint8*)tvb_memdup(wmem_file_scope(), tvb, 4, len));
+        proto_tree_add_item(tree, hf_dpaux_data, tvb, 4, len, ENC_NA);
 
     return 0;
 }
@@ -270,8 +263,7 @@ dissect_dpaux_from_sink(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                             len > 1 ? "s" : "");
         }
         proto_tree_add_uint(tree, hf_dpaux_len, tvb, 3, 1, len);
-        proto_tree_add_bytes(tree, hf_dpaux_data, tvb, 1, len,
-                            (guint8*)tvb_memdup(wmem_file_scope(), tvb, 1, len));
+        proto_tree_add_item(tree, hf_dpaux_data, tvb, 1, len, ENC_NA);
 
         if (transaction && transaction->is_native) {
             unsigned int k;
@@ -300,31 +292,30 @@ dissect_dpaux_from_sink(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 }
 
 static int
-dissect_dpaux(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
-        void *data _U_)
+dissect_dpaux(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data)
 {
-    /* Set up structures needed to add the protocol subtree and manage it */
     proto_item *ti;
     proto_tree *dpaux_tree;
+    gboolean from_source = FALSE;
+    struct dpaux_info *dpaux_info = (struct dpaux_info*)data;
 
-    struct dpaux_info *dpaux_info = (struct dpaux_info*)p_get_proto_data(
-        wmem_file_scope(), pinfo, proto_dpaux, 0);
+    if (dpaux_info != NULL)
+        from_source = dpaux_info->from_source;
 
     col_set_str(pinfo->cinfo, COL_PROTOCOL, "dpaux");
     col_set_str(pinfo->cinfo, COL_INFO, "DisplayPort AUX channel");
     col_set_str(pinfo->cinfo, COL_RES_DL_DST, "N/A");
 
-    if (dpaux_info->from_source)
+    if (from_source)
         col_set_str(pinfo->cinfo, COL_RES_DL_SRC, "DP-Source");
     else
         col_set_str(pinfo->cinfo, COL_RES_DL_SRC, "DP-Sink");
 
     /* create display subtree for the protocol */
     ti = proto_tree_add_item(tree, proto_dpaux, tvb, 0, -1, ENC_NA);
-
     dpaux_tree = proto_item_add_subtree(ti, ett_dpaux);
 
-    if (dpaux_info->from_source)
+    if (from_source)
         dissect_dpaux_from_source(tvb, pinfo, dpaux_tree);
     else
         dissect_dpaux_from_sink(tvb, pinfo, dpaux_tree);
@@ -408,7 +399,7 @@ proto_register_dpaux(void)
 
         { &hf_00000, { "DPCD_REV", "dpaux." "00000", FT_UINT8, BASE_HEX, NULL, 0, NULL, HFILL } },
         { &hf_00000_MINOR, { "MINOR", "dpaux." "00000" "_" "MINOR", FT_UINT8, BASE_HEX, NULL, 0x0f, NULL, HFILL } },
-        { &hf_00000_MAJOR, { "MAJOR", "dpaux." "00000" "_" "MAJOR", FT_UINT8, BASE_HEX, NULL, 0x0f, NULL, HFILL } },
+        { &hf_00000_MAJOR, { "MAJOR", "dpaux." "00000" "_" "MAJOR", FT_UINT8, BASE_HEX, NULL, 0xf0, NULL, HFILL } },
 
         { &hf_00001, { "MAX_LINK_RATE", "dpaux." "00001", FT_UINT8, BASE_HEX, NULL, 0, NULL, HFILL } },
         { &hf_00001_MAX_LINK_RATE, { "MAX_LINK_RATE", "dpaux." "00001" "_" "MAX_LINK_RATE", FT_UINT8, BASE_HEX, VALS(convert_link_rate), 0xff, NULL, HFILL } },
@@ -432,18 +423,13 @@ proto_register_dpaux(void)
     };
 
     /* Register the protocol name and description */
-    proto_dpaux = proto_register_protocol("DisplayPort AUX-Channel",
-            "DPAUX", "dpaux");
+    proto_dpaux = proto_register_protocol("DisplayPort AUX-Channel", "DPAUX", "dpaux");
     register_dissector("dpaux", dissect_dpaux, proto_dpaux);
 
     proto_register_field_array(proto_dpaux, hf, array_length(hf));
     proto_register_subtree_array(ett, array_length(ett));
 }
 
-void
-proto_reg_handoff_dpaux(void)
-{
-}
 
 /*
  * Editor modelines  -  https://www.wireshark.org/tools/modelines.html
diff --git a/epan/dissectors/packet-dpaux.h b/epan/dissectors/packet-dpaux.h
index e0ab61be03..828aba5687 100644
--- a/epan/dissectors/packet-dpaux.h
+++ b/epan/dissectors/packet-dpaux.h
@@ -11,8 +11,6 @@
 #ifndef PACKET_DPAUX_H
 #define PACKET_DPAUX_H
 
-extern int proto_dpaux;
-
 struct dpaux_info {
 	gboolean from_source;
 };
diff --git a/epan/dissectors/packet-dpauxmon.c b/epan/dissectors/packet-dpauxmon.c
index 13bba1daa0..f89fefd920 100644
--- a/epan/dissectors/packet-dpauxmon.c
+++ b/epan/dissectors/packet-dpauxmon.c
@@ -53,52 +53,59 @@ static const int *input_fields[] = {
 /* Initialize the subtree pointers */
 static gint ett_dpauxmon = -1;
 
+static const value_string packet_type_vals[] = {
+    { DPAUXMON_DATA, "Data" },
+    { DPAUXMON_EVENT, "Event" },
+    { DPAUXMON_START, "Start" },
+    { DPAUXMON_STOP, "Stop" },
+    { DPAUXMON_TS_OVERFLOW, "Timestamp Overflow" },
+    { 0, NULL }
+};
+
+static const value_string origin_vals[] = {
+    { 0, "Sink" },
+    { 1, "Source" },
+    { 0, NULL }
+};
+
 static int
 dissect_dpauxmon(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
         void *data _U_)
 {
     proto_item *ti;
     proto_tree *dpauxmon_tree;
-    guint8 packet_type = tvb_get_guint8(tvb, 0);
+    guint32 packet_type;
 
-    col_set_str(pinfo->cinfo, COL_PROTOCOL, "dpauxmon");
-    col_set_str(pinfo->cinfo, COL_INFO, "DisplayPort AUX channel");
+    col_set_str(pinfo->cinfo, COL_PROTOCOL, "DPAUXMON");
     col_set_str(pinfo->cinfo, COL_RES_DL_DST, "N/A");
     col_set_str(pinfo->cinfo, COL_RES_DL_SRC, "Internal");
 
     /* create display subtree for the protocol */
     ti = proto_tree_add_item(tree, proto_dpauxmon, tvb, 0, -1, ENC_NA);
-
     dpauxmon_tree = proto_item_add_subtree(ti, ett_dpauxmon);
 
-    proto_tree_add_uint(dpauxmon_tree, hf_packet_type, tvb, 0, 1, packet_type);
+    proto_tree_add_item_ret_uint(dpauxmon_tree, hf_packet_type, tvb, 0, 1, ENC_NA, &packet_type);
+    col_add_fstr(pinfo->cinfo, COL_INFO, "DisplayPort AUX channel - %s", val_to_str_const(packet_type, packet_type_vals, "Unknown"));
 
     switch (packet_type) {
     case DPAUXMON_DATA: {
-        struct dpaux_info *dpaux_info = (struct dpaux_info*)
-            wmem_alloc(wmem_file_scope(), sizeof(struct dpaux_info));
+        struct dpaux_info dpaux_info;
 
-        dpaux_info->from_source = tvb_get_guint8(tvb, 1);
-        p_add_proto_data(wmem_file_scope(), pinfo, proto_dpaux, 0, dpaux_info);
-        proto_tree_add_uint(dpauxmon_tree, hf_origin, tvb, 1, 1,
-                            dpaux_info->from_source);
+        dpaux_info.from_source = tvb_get_guint8(tvb, 1);
+        proto_tree_add_uint(dpauxmon_tree, hf_origin, tvb, 1, 1, dpaux_info.from_source);
 
-        call_dissector(dpaux_handle, tvb_new_subset_remaining(tvb, 2),
-                 pinfo, dpauxmon_tree);
+        call_dissector_with_data(dpaux_handle, tvb_new_subset_remaining(tvb, 2),
+                 pinfo, dpauxmon_tree, &dpaux_info);
         break;
         }
     case DPAUXMON_EVENT:
     case DPAUXMON_START:
-        col_set_str(pinfo->cinfo, COL_INFO,
-                    (packet_type == DPAUXMON_START) ? "Start" : "Input changed");
         proto_tree_add_bitmask(dpauxmon_tree, tvb, 1, hf_inputs, 0, input_fields,
                                ENC_BIG_ENDIAN);
         break;
     case DPAUXMON_STOP:
-        col_set_str(pinfo->cinfo, COL_INFO, "Stop");
         break;
     case DPAUXMON_TS_OVERFLOW:
-        col_set_str(pinfo->cinfo, COL_INFO, "Timestamp overflow");
         break;
     };
 
@@ -108,21 +115,6 @@ dissect_dpauxmon(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
 void
 proto_register_dpauxmon(void)
 {
-    static const value_string convert_packet_type[] = {
-        { DPAUXMON_DATA, "Data" },
-        { DPAUXMON_EVENT, "Event" },
-        { DPAUXMON_START, "Start" },
-        { DPAUXMON_STOP, "Stop" },
-        { DPAUXMON_TS_OVERFLOW, "Timestamp Overflow" },
-        { 0, NULL }
-    };
-
-    static const value_string convert_origin[] = {
-        { 0, "Sink" },
-        { 1, "Source" },
-        { 0, NULL }
-    };
-
     /* Setup protocol subtree array */
     static gint *ett[] = {
         &ett_dpauxmon
@@ -133,12 +125,12 @@ proto_register_dpauxmon(void)
     static hf_register_info hf[] = {
         { &hf_packet_type,
           { "Packet Type", "dpauxmon.packet_type",
-            FT_UINT8, BASE_DEC, VALS(convert_packet_type), 0,
+            FT_UINT8, BASE_DEC, VALS(packet_type_vals), 0,
             NULL, HFILL }
         },
         { &hf_origin,
           { "Origin", "dpauxmon.origin",
-            FT_UINT8, BASE_DEC, VALS(convert_origin), 0,
+            FT_UINT8, BASE_DEC, VALS(origin_vals), 0,
             NULL, HFILL }
         },
         { &hf_inputs,
@@ -169,8 +161,7 @@ proto_register_dpauxmon(void)
     };
 
     /* Register the protocol name and description */
-    proto_dpauxmon = proto_register_protocol("DPAUXMON DisplayPort AUX channel monitor",
-            "DPAUXMON", "dpauxmon");
+    proto_dpauxmon = proto_register_protocol("DPAUXMON DisplayPort AUX channel monitor", "DPAUXMON", "dpauxmon");
     proto_register_field_array(proto_dpauxmon, hf, array_length(hf));
     proto_register_subtree_array(ett, array_length(ett));
 }