Wireshark · Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 43908: /trunk/epan/dissectors/ /trunk/epan/dissectors/:

[Alexis La Goutte <alexis.lagoutte@xxxxxxxxx>]

Hi Martin,

It look good for me.

But you need to remove "ftp_data_tree = proto_item_add_subtree(ti, ett_ftp_data);", it is no longer use !

Also why use ftp and ftp-data, it is not better to use a ftp.data (filter) ? (and known ftp and ftp-data is not the same protocol...)

Alexis

On Mon, Jul 23, 2012 at 3:42 AM, Martin Mathieson <martin.r.mathieson@xxxxxxxxxxxxxx> wrote:

I think I'd like to change it to what the attached patch does.

i.e. have a quick look to see if the first few characters are printable.

- if yes, show the string (but as before not formatting more text than will be used)

- if no, just add the details of how many bytes are in the segment

In both cases, I append the details to the ftp-data root itself, rather than make that awkward call to proto_item_add_text().  This saves you opening up the tree (there is now nothing inside it).  And you can still make expressions like:

      ftp-data contains "PASS" 

      ftp-data contains 00:01:02

Unless anyone feels strongly about it I'll submit this in a day or 2.

Martin
 

On Sun, Jul 22, 2012 at 7:56 AM, Joerg Mayer <jmayer@xxxxxxxxx> wrote:

Maybe just calling packet-data is the right way to go here?

 ciao
      Jörg

On Sun, Jul 22, 2012 at 04:44:17AM +0000, martinm@xxxxxxxxxxxxx wrote:
> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=43908
>
> User: martinm
> Date: 2012/07/21 09:44 PM
>
> Log:
>  Calling tvb_format_text() for binary data segments (c1400 bytes) was
>  *very* slow (in a profiled run where FTP data is carried over LTE
>  MAC/RLC/PDCP/IP/TCP, this one function call was 20% of overall runtime).
>
>  Have limited to call to ITEM_LABEL_LENGTH, as that is all that
>  will be displayed anyway.  As per comment, I'm not convinced that doing
>  this for binary FTP data segments is worthwhile at all.  It doesn't
>  even display as hex.
>
> Directory: /trunk/epan/dissectors/
>   Changes    Path            Action
>   +6 -3      packet-ftp.c    Modified
>
> ___________________________________________________________________________
> Sent via:    Wireshark-commits mailing list <wireshark-commits@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-commits
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
>              mailto:wireshark-commits-request@xxxxxxxxxxxxx?subject=unsubscribe

--
Joerg Mayer                                           <jmayer@xxxxxxxxx>
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe