Re: proto_tree_add_subtree[_format]

[Anders Broman <anders.broman () ericsson com>]

-----Original Message-----
From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-bounces () wireshark org] On Behalf Of Alexis La 
Goutte
Sent: den 25 juni 2014 08:45
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] proto_tree_add_subtree[_format]

On Mon, Jun 23, 2014 at 12:21 AM,  <mmann78 () netscape net> wrote:
> In an effort to further remove proto_tree_add_text calls from the 
> Wireshark source, I've created proto_tree_add_subtree and 
> proto_tree_add_subtree_format.  The use case is to combine 
> proto_tree_add_text + proto_item_add_subtree into a single call when 
> the intent of the dissector is to create a "subtree with a label".  It 
> could also replace some proto_tree_add_none_format + 
> proto_item_add_subtree calls (some of the filters don't seem that 
> useful IMO and it appears that dissector just wanted to avoid 
> proto_tree_add_text).  proto_tree_add_subtree takes just a string 
> (const char*), while proto_tree_add_subtree_format supports printf style arguments (like proto_tree_add_text already 
> does).
> The intention is to have proto_tree_add_subtree be more optimized 
> since it doesn't need the printf style arguments (as a few API calls 
> have done that direction), but it's not currently implemented that way 
> (patches welcome!)
>
> I am currently in the process of converting applicable 
> proto_tree_add_text + proto_item_add_subtree into 
> proto_tree_add_subtree[_format] calls.  It should take me a while, so 
> any help would be appreciated (ideas on scripts would also be helpful, 
> as I haven't found an idea I like).  Just drop me a note so we're not 
> duplicating work. I'm going alphabetically by dissector filename (in 
> epan/dissectors directory), and I've already done the ASN.1 generated 
> dissectors (because 1 change counts twice).  I will not be touching 
> any of the proto_tree_add_none_format calls, as I probably won't have 
> enough knowledge of the dissector using it to be confident in removing the filter.
>
> I would also appreciate code reviews "encouraging" the use of this new API.
> We don't need to specifically revisit current patches (dissectors) 
> submitted to Gerrit (most will probably be committed before I get to 
> them in master anyway), but new ones should be subject to it.
>
> Michael
Hi Michael,

May be also add in epan/proto.h for proto_tree_add_subtree and proto_tree_add_subtree_format a 
G_GNUC_WARN_UNUSED_RESULT ?

Also it is no possible to add proto_tree_add_subtree_item ? with a hf item for top ? (with ftype FT_NONE for example)

Admittedly I haven't yet looked at what proto_tree_add_subtree() does but I was wondering if proto_tree_add_item() 
should be changed to take more argument like
If an "ett" is passed ( != NULL) a sub tree is returned or something like that? While at it, it should perhaps also 
return a value corresponding to the FT type?
Or is it better with separate functions? Internally the could be rolled into one perhaps.

> ___________________________________________________________________________
> 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
___________________________________________________________________________
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