Wireshark-dev: Re: [Wireshark-dev] proto.h extension

From: "John Dill" <John.Dill@xxxxxxxxxxxxxxxxx>
Date: Fri, 8 May 2015 10:06:51 -0400
>Message: 3
>Date: Thu, 7 May 2015 11:29:22 -0700
>From: Guy Harris <guy@xxxxxxxxxxxx>
>To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
>Subject: Re: [Wireshark-dev] proto.h extension
>Message-ID: <CF2E7490-F023-49C2-8383-6C1A1394B4E7@xxxxxxxxxxxx>
>Content-Type: text/plain; charset=iso-8859-1
>
>On May 7, 2015, at 8:13 AM, "John Dill" <John.Dill@xxxxxxxxxxxxxxxxx> wrote:
>
>> I have a couple of extensions that I created for the Wireshark baseline
>that we're using (1.10.x).  The diffs to proto.h and proto.c show the code
>changes to add a couple of features that I've found useful, unit strings
>and hiding the bits for bitmask header fields.
>> 
>> http://codepad.org/KTGdEL1t 
>
>You need more than
>
>	/* Following constants have to be ORed with a base_display_e when dissector
>	 * want to control aspects of the display string for a header_field_info */
>
>as a comment there - you need comments explaining what each of those flags actually *does*.

Guy,

Sorry it wasn't clear.  Starting from this snippet...

/*
 * BASE_UNIT_STRING - Append a unit string to the numeric value
 *
 * When active, Wireshark will append a unit string declared as a
 * simple 'char *' for the 'strings' to the numeric value.
 */

This will take a string in a header field with the following definition

{
  &hf_MFD_State_Data_Slew_Elevation_Ch1,
  {
    "Slew_Elevation_Ch1",
    "ndo.MFD_State_Data.Slew_Elevation_Ch1",
    FT_FLOAT,
    BASE_NONE | BASE_UNIT_STRING,
    "feet",
    0x0,
    NULL,
    HFILL
  }
},

and display a header field value with that text appended such as

Slew_Elevation_Ch1: 580 feet
                        ^^^^

Without it, I would have to construct a cumbersome sequence of
commands to add the item with proto_tree_add_item, use
proto_registrar_get_nth to access the header field, do the
conversion manually and finally use a proto_item_set_text to
manually add the unit string text in the display code.  This
further disassociates the units of a header field further away
from its definition.  With this in place, I can put the units
of the header field in the header_field_info struct (where it
belongs) and have a simple proto_tree_add_item and it just
works.

/*
 * BASE_NO_BITMASK_DISPLAY - Hide the bitfield display of a data
 *                           element.
 *
 * In Wireshark, any time the 'bitmask' argument is non-zero, a
 * bitfield display showing the location of 1's and 0's is
 * display is shown.  In certain situations, the display of
 * bitmask fields next to non-bitmask fields is quite jarring
 * because the display of these bitfields do not align the
 * data elements in an easy to scan column.
 *
 * 1010 .... Data Element 1
 * .... 0101 Data Element 2
 * Data Element 3
 *
 * If the 'bitmask' field is non-zero, one can append a
 * BASE_NO_BITMASK_DISPLAY bit to the 'display' parameter and
 * hide these bitfields.
 */

Normally, if you have a non-zero bitmask, you get a nice display
of a bitmask like the following:

0... .... .... .... .... .... = Parity: 0
.00. .... .... .... .... .... = SSM: Normal Operation (0)
...0 0000 00.. .... .... .... = Spare: 0

etc...

The problem is that in a lot of avionics data, the message data
is packed in a combination of bit fields and octet sized fields.
Without any method to disable the bitfield display, you get a
stuttered display of the data so that it looks like this.

00.. .... BitField 1: 0
..0. .... BitField 2: In-Air (0)
...0 0000 BitField 3: 0
Field 1: 0 feet
00.. .... .... .... BitField 4: 0
..00 0000 .... .... BitField 5: 0
.... .... 0... .... BitField 6: 0
.... .... .00. .... BitField 7: 0
.... .... ...0 0000 BitField 8: 0
Field 2: 20 degrees
Field 3: 45 degrees
0... .... BitField 9: 0
.0.. .... BitField 10: 0
..0. .... BitField 11: 0
...0 .... BitField 12: 0
.... 0000 BitField 13: 0
Field 4: 30 knots
...etc...

and so on in a message with several dozen to hundreds of data
elements.  The result is a very jagged display of that message's
data that looks ugly.  Some bit-fields are 8-bits wide, others
are 16-bits wide.

What BASE_NO_BITMASK_DISPLAY does is that when a bitmask field
is non-zero, it simply turns off the bitmask display for that
text string for the header field so that you get the following:

BitField 1: 0
BitField 2: In-Air (0)
BitField 3: 0
Field 1: 0 feet
BitField 4: 0
BitField 5: 0
BitField 6: 0
BitField 7: 0
BitField 8: 0
Field 2: 20 degrees
Field 3: 45 degrees
BitField 9: 0
BitField 10: 0
BitField 11: 0
BitField 12: 0
BitField 13: 0
Field 4: 30 knots

This flag avoids the ugly hack of having to access the header field
with proto_registrar_get_nth and basically having to format it myself.

Hopefully that better explains the motivation behind it.  If you
still have questions, let me know.  I have it implemented for 1.10.x
since that's what my development environment allows.  I'm mostly
curious whether this type of feature would be desired in the main
development branch.  I'm not asking anyone to do the work of porting
it as I'd do it when the need arose (or if I have time to burn).
It would be nice if eventually I do not need to maintain a separate
fork of Wireshark, or at least reduce the number of differences
between my fork and the main development branch.

Best regards,
John D.

<<winmail.dat>>