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 <
[email protected]> 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 <[email protected]> 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, [email protected] 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 <
>> [email protected]>
>> > Archives:    http://www.wireshark.org/lists/wireshark-commits
>> > Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
>> >              mailto:[email protected]
>> ?subject=unsubscribe
>>
>> --
>> Joerg Mayer                                           <[email protected]>
>> 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 <[email protected]>
>> Archives:    http://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>>              mailto:[email protected]
>> ?subject=unsubscribe
>
>
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <[email protected]>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:[email protected]
> ?subject=unsubscribe
>
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <[email protected]>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:[email protected]?subject=unsubscribe

Reply via email to