[mmann78@xxxxxxxxxxxx]

I still like the idea of the address types being "centrally registered" (in epan directory) and not put into the dissector code, mostly because many of the address types are used in multiple dissector/protocols.  You may also introduce epan dependencies (like from proto.c) with the dissectors, and I think that should be avoided (yes I know some exist, but I've been working to slowly remove them).   I also like the idea of keeping the AT_ enums to prevent dissectors from including (IMO unnecessary) header files with the "get the address type I need" functions.  Having just attempted to add a few new FT_ types, I don't think that process was that bad. 

 

I just really like the idea that a series of properly structured function callbacks could eliminate a lot of the sprintf processing of the "address to string" and drastically improve performance (since almost all packets do some time of address to string processing).  Also included could be standards in displaying "address string" + "address name resolution" formatting.  Personally I like the %s (%s), for address string then resolution, but I feel dissectors do this inconsistently because we still have separate functions for each (i.e. address_to_str() and  get_ether_name()), and those are usually called within a printf style function themselves.

 

 

 

-----Original Message-----
From: Guy Harris <guy@xxxxxxxxxxxx>
To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
Sent: Fri, Jan 16, 2015 2:00 pm
Subject: Re: [Wireshark-dev] address to string optimization

On Jan 16, 2015, at 6:04 AM, mmann78@xxxxxxxxxxxx wrote:

> A few weeks ago I stumbled across the following comment in address_to_str.c:
>  
> /*XXX FIXME the code below may be called very very frequently in the future.
>   optimize it for speed and get rid of the slow sprintfs */
> /* XXX - perhaps we should have individual address types register
>    a table of routines to do operations such as address-to-name translation,
>    address-to-string translation, and the like, and have this call them,
>    and also have an address-to-string-with-a-name routine */
> /* XXX - use this, and that future address-to-string-with-a-name routine,
>    in "col_set_addr()"; it might also be useful to have address types
>    export the names of the source and destination address fields, so
>    that "col_set_addr()" need know nothing whatsoever about particular
>    address types */
> /* convert an address struct into a printable string */
>  
>  
> This all sounded like a very good idea (mostly the removal of the sprintfs), but there was a lot of "prep" work involved to make "address registering" easier to create (the "to string" function were a bit scattered/haphazard).  I believe the "prep" work is now complete.  The problem I have is that I can't quite get my head around what's needed for "address registering".  I assume it would be modeled loosely after field type registration.
>  
> If the author of the comment is still around, I would gladly work with them to try to come up with address registering.  Mostly what I'm volunteering for is "filling out the tables" once a few address types have been figured out/added.  This also seems to be a preferred solution to the current USB addressing (compare) issues.

I think the author of the comment is named "Ronnie Sahlberg+Guy Harris+possibly some others". :-)

I.e., I think Ronnie's the author of the first part of the comment, and I might have been the author of the second and third parts, but I'm not sure.

"Address type registration", at least as I'd do it, would allow a dissector to register an address type with those routines and get, in response, a numerical value to use as the type field in an address structure.  This would be a bit different from the way field types *currently* work - currently, a fixed set of FT_ values are defined in an enum in epan/ftypes/ftypes.h, rather than being numerical values assigned at startup time.

So the AT_ enums defined in epan/address.h would no longer be defined there.  Yes, this means that all the case statements and if statements checking for AT_ values would have to be changed, perhaps by virtue of having additional functions provided for registered address types, but that's arguably a Good Thing, as it means fewer places would need to be changed if a new address type were introduced.
___________________________________________________________________________
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