Re: switching to proto_tree_add_subtree()

[darkjames-ws () darkjames pl]

Hi,

On Tue, Jul 29, 2014 at 08:33:57PM +0200, Martin Kaiser wrote:
> I'm confused about this block in TRY_TO_FAKE_THIS_ITEM_OR_FREE
>
>         if (!(PTREE_DATA(tree)->visible)) { \
>                 if (PTREE_FINFO(tree)) { \ ### [1] Sake workaround for some bugs (details: 00c05ed3f)
>                         if ((hfinfo->ref_type != HF_REF_TYPE_DIRECT) \
>                             && (hfinfo->type != FT_PROTOCOL || \
>                                 PTREE_DATA(tree)->fake_protocols)) { \
>                                 free_block; \
>                                 /* just return tree back to the caller */\
>                                 return tree; \
>
> If tree is not visible (and fake_protocols is set, which seems to be the
> default), we return the tree itself.
>
> proto_item *it = proto_tree_add_text(tree, tvb, 0, -1, "foobar");
>
> If tree!=NULL && !(PTREE_DATA(tree)->visible) the return value it==tree
> Why does this make sense?

Ok, what value you propose to return instead of 'tree'?

IMHO we could return:

 a/ tree root (right now not so good idea cause of [1] line)
 b/ some fake item.
 <strike>c/ NULL</strike> (you cannot cause it will make filtering stop working).


Right now AFAIK some API should be carefully used when we're faking tree - like proto_item_set_len() or 
proto_item_append_string() (it might be can of open bugs),

Still, currently it works quite well, isn't it?
___________________________________________________________________________
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