Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 5881] Media Independent handover (MIH) protocol dissector

[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5881

--- Comment #17 from Jaap Keuter <jaap.keuter@xxxxxxxxx> 2011-12-05 16:27:44 CET ---
(In reply to comment #15)
> Created an attachment (id=7525)
 --> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=7525) [details]
> Made changes to the patch according to the comments
> 
> Thank you for your quick review, the following changes are done- 
>...
> -->Have a look at proto_tree_add_boolean_bits_* for bitfield dissection.
>         -unable to find the function in docs, also can you tell me where I have
> to replace the function with the suggested function?
> 

See epan/proto.h

> -->You could use more proto_tree_add_item's i.s.o. proto_tree_add_text
>         -I am sorry but I did not understand the meaning of i.s.o, also is
> there any problem in using proto_tree_add_text?

proto_tree_add_item has the big disadvantage of not providing filterable
fields, so users can't filter and/or search for these fields/values.

> -->Using symbols like MIHF_ID, i.s.o. 52, make the source more readable
>         -I did not understand the meaning of this suggestion.

Compare this:
-------------------8<-------------------
    switch (hahshudf)
    {
        case 52 :
            proto_tree_add_item(tlv_tree, hf_bygfb, tvb, offset+1, 1, FALSE);
            break;

        case 3 :
            proto_tree_add_item(tlv_tree, hf_asdfa, tvb, offset, 1, FALSE);
            break;

        case 4 :
            proto_tree_add_item(tlv_tree, hf_bvaybyf4, tvb, offset, 1, FALSE);
            break;

        case 5 :
            temp_tree = proto_tree_add_text(tlv_tree, tvb, offset, 4, "list of
events - ");
            pasdfjv_list = tvb_get_ntohl(tvb, offset);
            dissect_pasdfjv_list(temp_tree, pasdfjv_list);
-------------------8<-------------------
To this:
-------------------8<-------------------
#define STATUS          3
#define LINK_TYPE       4
#define MIH_EVT_LIST    5
#define MIHF_ID        52


    switch (type)
    {
        case MIHF_ID:
            mihf_id_len = tvb_get_guint8(tvb, offset);
            proto_tree_add_item(tlv_tree, hf_mihf_id, tvb, offset+1,
mihf_id_len, FALSE);
            break;

        case STATUS:
            proto_tree_add_item(tlv_tree, hf_status, tvb, offset, 1, FALSE);
            break;

        case LINK_TYPE:
            proto_tree_add_item(tlv_tree, hf_link_type, tvb, offset, 1, FALSE);
            break;

        case MIH_EVT_LIST:
            temp_tree = proto_tree_add_text(tlv_tree, tvb, offset, 4, "list of
events - ");
            mih_evt_list = tvb_get_ntohl(tvb, offset);
            dissect_evt_list(temp_tree, mih_evt_list);
-------------------8<-------------------

Which one is more readable? I'll guess you'll say the second. 
That's why using symbols is recommended over literals.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.