Wireshark-dev: Re: [Wireshark-dev] switching to proto_tree_add_subtree()
Date Index · Thread Index · Other Months · All Mailing Lists
Date Prev · Date Next · Thread Prev · Thread Next
From: Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>
Date: Wed, 30 Jul 2014 16:20:38 -0400
On 07/30/14 14:02, darkjames-ws@xxxxxxxxxxxx wrote:

On Wed, Jul 30, 2014 at 11:23:55AM -0400, Jeff Morriss wrote:

On 07/30/14 01:30, darkjames-ws@xxxxxxxxxxxx wrote:

On Tue, Jul 29, 2014 at 06:47:53PM -0400, Jeff Morriss wrote:



Does this mean that this code in add_subtree_format() should be
setting *tree_item to 'tree' (instead of NULL) here:


    /* Make sure pi is initialized in case TRY_TO_FAKE_THIS_ITEM bails */
        if (tree_item != NULL)
                *tree_item = NULL;


Wouldn't be it simpler if we just remove 'tree_item' argument?
Cause I understand that after change *tree_item is always the same as return value?


Well, no, they'll only return the same thing if
TRY_TO_FAKE_THIS_ITEM short-cuts us out, right?


1138 proto_tree_add_subtree_format(proto_tree *tree, tvbuff_t *tvb, gint start, gint length, gint idx, proto_item **tree_item, const char *format, ...     )
...
1159         pt = proto_item_add_subtree(pi, idx);
1160         if (tree_item != NULL)
1161                 *tree_item = pi;
1162
1163         return pt;

And proto_item_add_subtree(pi, ...) returns pi, so *tree_item == pt == pi.

Anyway, sorry, I'm fine with this API.
If you want to ever make proto_item != proto_tree without even more refactoring it makes sense.


https://code.wireshark.org/review/3271