Wireshark-bugs: [Wireshark-bugs] [Bug 2981] Patch to add extension support to the X11 dissector

From: bugzilla-daemon@xxxxxxxxxxxxx
Date: Tue, 3 Mar 2009 10:44:12 -0800 (PST)

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2981





--- Comment #15 from Peter Harris <peter.harris@xxxxxxxxxxxxxxx>  2009-03-03 10:44:10 PDT ---
(In reply to comment #14)
> (In reply to comment #13)
> > I've uploaded a half a dozen .pcap files to the Wiki's SampleCaptures page.
> > Fuzz testing over those files turned up an infinite loop in GLX/Render when the
> > length field on the wire is invalid. Fixed in v4 of the patch.
> 
> Excellent.  But I don't think this is a good way to fix it:
...
> +           REPORT_DISSECTOR_BUG("Invalid glRender length");

> It's not a dissector bug that the packet is malformed.  You should either
> return or force an exception to be thrown (by trying to read beyond the end of
> the tvb).

I'd like to throw an error, so it shows up in red in wireshark. I need to read
past the end of the tvb to do so? Ick.

I'll make this change for v5.

> > v4 also uses "static _U_" for a few functions that were previously non-static.
> 
> I assume those functions are unused because xcb or whatever defines the
> structures but doesn't use them?

Well, xcb does use them - in the core protocol. I wanted to leave the core
protocol dissector alone for now.

>  [Since this is generated code] why not just
> leave them out of the dissector until they become used?

Right now, I process the protocol description in a streamy and (mostly)
stateless manner. Changing the generator to emit structures lazily would be a
rather large change.

> - why abuse value_string?
...
> It would be nicer to just define your own structure to hold pointers to
> functions.

Yes, it would. I thought I was using value_string lookup functions, but it
appears not. I'll make this change for v5.

> - (Minor, but...) since this is generated code, why not get the indenting
> right?

I didn't do this initially because this is generated code - nobody should be
editing it by hand anyway.

To fix up the indenting I'll have to pass an extra parameter around (since the
same function generates code at different indent levels).

I'll make this change for v5.

> I may have more comments later--there's a lot of code here.

Thank you for your review. The code is much better already.


-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.