Wireshark mailing list archives
Re: proto_tree_add_subtree[_format]
From: mmann78 () netscape net
Date: Wed, 25 Jun 2014 08:18:13 -0400 (EDT)
I have no objection to adding G_GNUC_WARN_UNUSED_RESULT to proto_tree_add_subtree[_format] as it appears to already be used on proto_item_add_subtree. The intent of proto_tree_add_subtree[_format] is more directed at trying to remove proto_tree_add_text calls. The use of proto_tree_add_text seems to fall into (about) 3 categories 1. Display field value (which should be converted to proto_tree_add_<something_filterable>) 2. Display "expert info" (which should be converted to use expert info API) 3. Dissector needs a "labeled subtree". This seems like the only legitimate use, but opens up the possiblily of (ab)using it for the other two, so that's why its being replaced with proto_tree_add_subtree[_format] so there is no legitimate use of proto_tree_add_text. This was not an attempt to create "superfunctions" or eliminate proto_item_add_subtree, which is how I interpret your proto_tree_add_item suggestions. Michael -----Original Message----- From: Anders Broman <anders.broman () ericsson com> To: alexis.lagoutte <alexis.lagoutte () gmail com>; Developer support list for Wireshark <wireshark-dev () wireshark org> Sent: Wed, Jun 25, 2014 4:05 am Subject: Re: [Wireshark-dev] proto_tree_add_subtree[_format] -----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
___________________________________________________________________________ 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
Current thread:
- proto_tree_add_subtree[_format] mmann78 (Jun 22)
- Re: proto_tree_add_subtree[_format] Alexis La Goutte (Jun 24)
- Re: proto_tree_add_subtree[_format] Anders Broman (Jun 25)
- Re: proto_tree_add_subtree[_format] mmann78 (Jun 25)
- Re: proto_tree_add_subtree[_format] Anders Broman (Jun 25)
- Re: proto_tree_add_subtree[_format] Alexis La Goutte (Jun 24)