Wireshark · Wireshark-dev: Re: [Wireshark-dev] proto_tree_get_parent()

["Maynard, Chris" <Christopher.Maynard@xxxxxxxxx>]

Understood.  But although it "works well and as expected" as Tomas suggests, it's not very efficient and still confusing.  Perhaps some sort of comment about it like, "the parent of a tree is an item which is the tree itself. proto_item and proto_tree are both proto_node."[1] would help make understanding clearer and avoid future confusion.
 
And it's inefficient because it could be more succinctly written as the following:

proto_item*
proto_tree_get_parent(proto_tree *tree) {
        return (proto_item*) tree;
}

Or even as a macro in proto.h for example:
#define proto_tree_get_parent(t)        ((proto_item *)(t))

- Chris
[1] http://www.wireshark.org/lists/wireshark-dev/201003/msg00320.html


-----Original Message-----
From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of wsgd
Sent: Tuesday, March 30, 2010 3:06 PM
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] proto_tree_get_parent()

If I well understand the current Wireshark design on this point :

1) the parent of an item is the tree which contains this item
And a tree is an item.
-> proto_item_get_parent

2) the parent of a tree is an item which is the tree itself
proto_item and  proto_tree are both proto_node.
-> proto_tree_get_parent



Olivier


Maynard, Chris a écrit :
> ... and if we really want to retain a function call for whatever reason, then at the very least it should be renamed to something like proto_tree_cast_tree_to_item() for clarity, since that's all the function does.
>
> - Chris
>
>
> -----Original Message-----
> From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Maynard, Chris
> Sent: Monday, March 29, 2010 10:21 AM
> To: 'Developer support list for Wireshark'
> Subject: Re: [Wireshark-dev] proto_tree_get_parent()
>
> I still don't get it.  Compare proto_item_get_parent() with proto_tree_get_parent():
>
> proto_item*
> proto_item_get_parent(proto_item *ti) {
>         if (!ti)
>                 return (NULL);
>         return ti->parent;
> }
>
> proto_item*
> proto_tree_get_parent(proto_tree *tree) {
>         if (!tree)
>                 return (NULL);
>         return (proto_item*) tree;
> }
>
> The former returns a pointer to the parent whereas the latter simply casts the tree to an item.  If a cast is all that's needed, then that can be very easily accomplished without a confusing and misleading function call.
>
> - Chris
>
>
> -----Original Message-----
> From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Kukosa, Tomas
> Sent: Monday, March 29, 2010 2:26 AM
> To: Developer support list for Wireshark
> Subject: Re: [Wireshark-dev] proto_tree_get_parent()
>
> Hi, 
> I guess the function works well and as expected.
> It returns parent proto_item of the proto_tree and in our internal representation the item and its subtree are the same object only casted either to proto_item or proto_tree.
>
> Regards,
>   Tomas
>
>
> -----Original Message-----
> From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Maynard, Chris
> Sent: Sunday, March 28, 2010 1:46 AM
> To: 'Developer support list for Wireshark'
> Subject: Re: [Wireshark-dev] proto_tree_get_parent()
>
> This sure looks like a bug to me.  A quick search shows at least these 18 dissectors calling it:
> $ grep -l proto_tree_get_parent packet-*.c
> packet-amr.c
> packet-assa_r3.c
> packet-dcerpc.c
> packet-h264.c
> packet-h450-ros.c
> packet-h450.c
> packet-h460.c
> packet-iax2.c
> packet-isakmp.c
> packet-mikey.c
> packet-mp4ves.c
> packet-q931.c
> packet-q932-ros.c
> packet-qsig.c
> packet-rtp.c
> packet-smb2.c
> packet-ssl.c
> packet-tcp.c
>
> If things are working correctly in those dissectors, then I think it's by pure luck and the function call can simply be omitted.  Either way, I think the function should be changed to implement what its proto_item_get_parent() peer implements.  I would suggest filing a bug report about.
>
> Good catch.
> - Chris
>
>
> -----Original Message-----
> From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Eloy Paris
> Sent: Thursday, March 25, 2010 8:49 PM
> To: Developer support list for Wireshark
> Subject: [Wireshark-dev] proto_tree_get_parent()
>
> Is this right (from epan/proto.c):
>
> proto_item*
> proto_tree_get_parent(proto_tree *tree) {
> 	if (!tree)
> 		return (NULL);
> 	return (proto_item*) tree;
> }
>
> This basically returns the same thing that is received as a parameter. 
> Shouldn't tree->parent be returned instead, just as 
> proto_item_get_parent() (a few lines above in the same file) does?
>
> The weird thing is that there is code using this seemingly broken 
> proto_tree_get_parent(). I don't know how things are working given that 
> proto_tree_get_parent() is not really providing the parent node.
>
> Confused,
>
> Eloy Paris.-
> netexpect.org
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
> CONFIDENTIALITY NOTICE: The contents of this email are confidential
> and for the exclusive use of the intended recipient. If you receive this
> email in error, please delete it from your system immediately and 
> notify us either by email, telephone or fax. You should not copy,
> forward, or otherwise disclose the content of the email.
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
> CONFIDENTIALITY NOTICE: The contents of this email are confidential
> and for the exclusive use of the intended recipient. If you receive this
> email in error, please delete it from your system immediately and 
> notify us either by email, telephone or fax. You should not copy,
> forward, or otherwise disclose the content of the email.
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
> CONFIDENTIALITY NOTICE: The contents of this email are confidential
> and for the exclusive use of the intended recipient. If you receive this
> email in error, please delete it from your system immediately and 
> notify us either by email, telephone or fax. You should not copy,
> forward, or otherwise disclose the content of the email.
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
>
>
>   


-- 
Wireshark Generic Dissector http://wsgd.free.fr

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
CONFIDENTIALITY NOTICE: The contents of this email are confidential
and for the exclusive use of the intended recipient. If you receive this
email in error, please delete it from your system immediately and 
notify us either by email, telephone or fax. You should not copy,
forward, or otherwise disclose the content of the email.