Re: address to string optimization

[mmann78 () netscape net]

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 () alum mit edu>
To: Developer support list for Wireshark <wireshark-dev () wireshark org>
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 () netscape net 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 () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

 

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