On 11/26/20 11:03 AM, John Thacker wrote: > > On Thu, Nov 26, 2020 at 1:19 PM Maynard, Christopher via Wireshark-dev > <wireshark-dev@wireshark.org <mailto:wireshark-dev@wireshark.org>> wrote: > > Many protocols contain subtrees, such as a header with various fields > that are part of the header, and it’s convenient/logical to group those > fields within the header subtree. However, doing so results in a Packet > Diagram that only shows the raw bytes of the subtree rather than the > individual fields contained within the subtree. > > So either I’m doing something wrong, in which case I welcome any > suggestions for improving the display, or there seems to be a current > limitation to the way the Packet Diagram behaves with respect to subtrees. > Has anyone else noticed this? > ... > > Is there a way to achieve this while still grouping the fields within a > subtree? > > > Not in a subtree currently. If you look around line 600 of > ui/qt/packet-diagram.cpp, you'll see that it only groups the top level fields > in each protocol.
That's correct. I wanted to keep the initial implementation as simple, naive, and lazy as possible. > For the same reason, bitmask fields that are grouped together not in a > subtree, using proto_tree_add_bitmask_list() > (like packet-rtp.c#L2072 with octet1_fields), then they are displayed > separately (in master, post commit > https://gitlab.com/wireshark/wireshark/-/commit/7654bb260d08fdb7adeafce1877fa3c38f3465ae > > <https://gitlab.com/wireshark/wireshark/-/commit/7654bb260d08fdb7adeafce1877fa3c38f3465ae> > ), whereas > for bitmask fields that are added with a subtree with > proto_tree_add_bitmask() only the top level header > item appears. > > You can see some images here: > https://gitlab.com/wireshark/wireshark/-/merge_requests/959 > <https://gitlab.com/wireshark/wireshark/-/merge_requests/959> > There you can see bitmask fields that are displayed properly because there is > no subtree. > > I agree it would be a nice enhancement to travel down into the children of > items that have children, though I imagine > you'd have to take care in some cases; e.g., dissect_e164_msisdn() from > packet-e164.[ch] is used a lot in various dissectors, > and has a header that has the entire number, with child that only has the > country code (but not a child for the non country code digits). > The simplest way to descend into the subtree for a E.164 number would thus > only has an entry for the country code but leave the > other bits blank. Or you could have issues with dealing with overlaps. Would it make sense to add second-level items only if they collectively fit the offset+size of the top-level item? In this case we'd skip the second-level country code child, but we'd add it if dissect_e164_msisdn() added a non-country code sibling field. BTW, for 1- and possibly 2-bit fields we might want to take inspiration from https://twitter.com/vivekrj/status/1269649718059118601 and rotate the label 90 degrees. ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
