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

[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

Hi Chris,

Please don't confuse semantics with syntaxis.

Thanks,
Jaap

Send from my iPhone

On 29 mrt 2010, at 16:33, "Maynard, Chris" <Christopher.Maynard@xxxxxxxxx 
> wrote:

> ... 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