Wireshark-dev: Re: [Wireshark-dev] Trying to submit a patch

From: Alexis La Goutte <alexis.lagoutte@xxxxxxxxx>
Date: Sat, 5 Sep 2015 12:48:06 +0200


On Fri, Sep 4, 2015 at 9:15 PM, John Dill <John.Dill@xxxxxxxxxxxxxxxxx> wrote:

>Message: 3
>Date: Tue, 1 Sep 2015 09:45:14 -0400
>From: Hadriel Kaplan <the.real.hadriel@xxxxxxxxx>
>To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
>Subject: Re: [Wireshark-dev] Trying to submit a patch
>Message-ID:
>        <CALqNqbJMmcvQQbnOC1=dSHhx-pWTWjA+s29C9j6ooNye3baDuA@xxxxxxxxxxxxxx>
>Content-Type: text/plain; charset=UTF-8
>
>I just tried pushing with https, and it works. But of course you'll
>still need a Change-ID and such. What *exact* error message are you
>seeing?
>
>Also, why would you submit something new into master-1.12?  We only do
>bug fixes for 1.12, not new features. New features go in master (i.e.,
>1.99.x).

(just replying to the list in general)

I'm still in the process of trying to upgrade my dissector to Wireshark 1.99 (~150kloc).  I've got the proto.[ch] files updated but not well tested, but I'm having issues with how to replace some of my uses of 'proto_item_add_text' since it's been deprecated.

For example, I have some ASCII art that I use to decorate certain things.  One example is packets that contain a 15x24 grid of character data, so I have statements like

proto_tree_add_text(page_tree, tvb, offset, 0, " -------------------------- ");

or

proto_tree_add_text(page_tree, tvb, line_offset, Display_Line_Width, "| %s |", Display_Data);

to create a subtree that expands into a display page like

 -----------------------
|      line 1 text      |
|      line 2 text      |
|      line 3 text      |
|           ...                |
|      line 15 text    |
 -------------------------

instead of just a bunch of individual characters (and their attributes) in a big vertical list (1080 header fields to be exact), with subtrees for each character's attribute, grouped in a subtree by horizontal line.  It was untenable to determine what the screen display was without resorting to doing something like this.

I'm not quite sure how to replace these with something not deprecated (and README.dissector doesn't even document it anymore!).  I really don't want to have to create a bunch of dummy header fields to create the page view and don't want them to be associated with a header field in a filter _expression_.  The proto_tree_add_text function really fit this use-case well.
Use proto_tree_add_string...
 

In other instances, I just want to assign a label to an undissectable block of bytes in a packet that I do not want to be filtered on, where I'd use:

proto_tree_add_text(pdu_tree, tvb, offset, pdu_payload_length, "Data Payload");
Use proto_tree_add_item with hf use FT_NONE

Now, all field will be filtereable...
(it is possible to reuse the same hf for multiple field)
 

This shows up in the packet details pane and I can click on that label and it would highlight the bytes in the packet bytes pane that that block refers to.  I have lots of these "Data Payload" since I do not have complete documentation of the system, or it may be used to read a data file from another aircraft that share some messages but have others unprocessed.

It seems clunky to have to create header fields for each one of these undissectable blobs, and I already have enough bogus "Spare" and "Pad" header fields in the stuff I do know about to the point that it takes 8-10 seconds to open up my specific protocol in the filter _expression_ dialog.

I need some suggestions on how best to approach this in 1.99.x?  I've adapted all the uses where I used it for subtrees, but these items still stand out.  I suppose if it ever gets removed, I can keep the 'proto_tree_add_text' function in my patched version of Wireshark.  How hard would it be to petition to keep this function around, either with its current name or a new name?

Thanks,
John D.

P.S. Also, when reading the documentation in README.heuristic, the examples use the old prototype for heur_dissector_add.
on 1.99 branch ?
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe