On 07/29/14 15:39, darkjames-ws@xxxxxxxxxxxx wrote:
On Tue, Jul 29, 2014 at 09:18:18PM +0200, darkjames-ws@xxxxxxxxxxxx wrote:
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'?
<strike>c/ NULL</strike> (you cannot cause it will make filtering stop working).
Ah! We could do if (it == NULL) { it = tree; } when creating subtree, for leaf we don't need a fix.
It still lot of work to do, but I'm +0.5 for it ;-)
Going back to Martin's initial problem...
However, I don't quite understand why for tree!=NULL but not visible,
proto_tree_add_text() returns tree. I can see this in the code, we call
TRY_TO_FAKE_THIS_ITEM(), which returns the tree itself when it's not
visible. But what sense does this make for the caller?
I think it's because while proto_trees are allowed to be NULL, we don't
want (or the API is not ready for) proto_items to be NULL. (IOW that
"return tree" quoted above is (ab)using the fact that a proto_item is a
proto_tree.)
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;