Wireshark-bugs: [Wireshark-bugs] [Bug 4481] Update x11 dissector to support new XCB features
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=4481
--- Comment #5 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2010-04-17 14:24:42 PDT ---
(In reply to comment #2)
> The point of generating all of this automatically from upstream protocol
> descriptions is so you don't have to review every single line. The X11 protocol
> isn't small, and it isn't shrinking.
>
> The manual changes amount to:
>
> epan/dissectors/packet-x11.c | 14 +
> tools/process-x11-xcb.pl | 323
> ++++++++++++++++++++++++++++---------------
> 2 files changed, 228 insertions(+), 109 deletions(-)
>
> Which should be a lot easier to review, in theory.
True, but that supposes I understand the Perl code (and/or the upstream stuff)
well enough to predict what it'll generate ;-).
For example, this code here isn't great because we could loop many, many times
without throwing an exception (if count is very large):
+static void struct_KeyName(tvbuff_t *tvb, int *offsetp, proto_tree *root, int
little_endian, int count)
+{
+ int i;
+ for (i = 0; i < count; i++) {
+ proto_item *item;
+ proto_tree *t;
+
+ item = proto_tree_add_item(root, hf_x11_struct_KeyName, tvb, *offsetp,
1, little_endian);
+ t = proto_item_add_subtree(item, ett_x11_rectangle);
+ listOfByte(tvb, offsetp, t, hf_x11_struct_KeyName_name, 4,
little_endian);
+ }
+}
(note the lack of an offset increment which would otherwise save us)
Other than that, the patch looks good.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
