Wireshark-dev: Re: [Wireshark-dev] Wireshark-dev Digest, Vol 136, Issue 14
From: John Dill <[email protected]>
Date: Tue, 12 Sep 2017 13:45:51 +0000
>Date: Mon, 11 Sep 2017 17:57:18 -0400
>From: Michael Mann <[email protected]>
>To: [email protected]
>Subject: Re: [Wireshark-dev] unit_name_string for FT_STRING field
>        types?
>
>Are you suggesting "unit types" for "strings" or are you suggesting "unit types" for "string values that should really be considered >integers or floats"?

I suppose I'm suggesting allowing unit strings for FT_STRING when the content matches something like this "^[-+]?[0-9]*\.?[0-9]+$" or "^[-+]?[0-9]+".  Granted, I'm likely the only one who has a use case that cares about such things.

>It certainly sounds like the latter and in which case I would suggest converting them in your dissector.  Numeric fields that are >treated as numbers have more flexibility with comparison and math operations.

Can you explain what you mean in more detail as it sounds good but I'm not sure how to go about this?  How do you dissect a number represented as an ASCII string in a packet's data but interpret it as a FT_INT32 or FT_FLOAT type, since those allow BASE_UNIT_STRING?  Even though the data is ASCII characters, it's still a number and it has units (defined in an outside document).  Do you create a tvbuff that contains a modified buffer containing actual integers or floating point numbers of the data and then pass that tvb to a proto_tree_add_item of the appropriate type?  I don't know how I'd maintain the link between the header field and the corresponding ASCII text in the packet details pane, but still interpret it as a kind of FT_INT or FT_FLOAT type?

I don't think there's interest in filters that allow real numeric comparisons against the strings, though that would be cool.  Right now it's display the ASCII string and units.  Lumping in a unit_name_string in the 'strings' would be cleaner from a maintenance perspective if I can put it in a FT_STRING types header_field_info object.  I could expose the unit_name_string_get functions in proto.h and convert the ASCII strings to local integer or double types as its dissected, but I'd still need to maintain a separate table of header fields -> unit_name_string objects in my dissector.

>To me there isn't an argument here to have support for "true" strings and the proto_tree_add_string_format or >proto_tree_add_string_format_value seems more appropriate.

I agree that in general for FT_STRING, one should use proto_tree_add_string_format and friends.  I just have a use case where there's just enough repetition that making the dissector "ugly" for several dozen fields when I can patch Wireshark to make it look cleaner (and more maintainable to others) is an argument I'm having with myself.

I'm not intending to push this as a feature to be integrated into the Wireshark main, just how to modify the source enough to see if I can make it work for this use case.  I already have a modified Wireshark repo for some minor extensions already (BASE_SUPPRESS_BITFIELD to turn off those bitfield displays for certain bitfields, and I have a "word" size equivalent of the bytestring_to_str and a tvb_bytes_to_str_punct for 1553 traffic over ethernet).

I'm just hunting and pecking for something that makes the dissector a bit easier to maintain.

Best regards,
John Dill