Wireshark-dev: Re: [Wireshark-dev] LLRP dissector support
From: "Poduska, Matt" <Matt.Poduska@xxxxxxxxxxxx>
Date: Mon, 7 Jul 2008 12:04:25 -0500
I've rewritten the guts of this dissector to remove all of the buffer portability abstraction. A new patch has been added to the bugzilla case. Please let me know if it's acceptable. Thanks, Matt Poduska -----Original Message----- From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Jaap Keuter Sent: Tuesday, April 08, 2008 10:59 AM To: mpoduska.intermec@xxxxxxxxx; Developer support list for Wireshark Subject: Re: [Wireshark-dev] LLRP dissector support Hi, I have to refer back to my earlier statement: "the code is very hard to read. I can't really comment beyond that.". What John and you are asking is that we actually read it, understand it and see how this could be accepted. That is the whole point. If you would give it another go we are willing to give it a shot, just like any other dissector. Thanx, Jaap Matt Poduska wrote: > Is there anything other than the use of the portability wrappers that > are preventing this dissector from being accepted (making the code > very hard to read and maintain)? > > Please let me know what needs to change in the dissector in order to > be accepted. > > - Matt Poduska > > -----Original Message----- > From: wireshark-dev-bounces@xxxxxxxxxxxxx > [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Jaap Keuter > Sent: Tuesday, April 08, 2008 1:17 AM > To: Developer support list for Wireshark > Subject: Re: [Wireshark-dev] LLRP dissector support > > Hi, > > Well, as the general comment states "the code is very hard to read". I > can't really comment beyond that. > If the code is reasonably written and understandable and adheres to > the coding guidelines found in README.developer it shouldn't be a big > problem getting it in. > > Thanx, > Jaap > > John R. Hogerhuis wrote: >> Jaap Keuter <jaap.keuter@...> writes: >> >>> Hi John, >>> >>> I've been looking at this submission from the start, and frankly I >>> don't like it. It is like Ronnie says in >>> http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1957#c4, this >>> code is very hard to read, let alone maintain. >>> I don't want to sign off on that and burden myself and other with >>> the maintenance chores. So I left it alone for another core >>> developer to eventually pick it up. It seems none is confident enough to commit it. >>> >> Well that's a clear statement of the problem, thanks for the reply. >> >> It appears Matt is responding favorably to requests to make specific >> improvements. General criticisms about hard to read/maintain and how >> he has abstracted the message parsing are obviously harder to address. >> My understanding is that parts of the code are generated based on XML >> descriptors of the binary protocol available from >> http://sourceforge.net/projects/llrp-toolkit (I am a developer for >> this > project but not the LLRP dissector). >> If the code could be simplified to avoid wrappers are there other >> issues for you or Ronnie that would stand in the way of commit? >> >> Thanks, >> >> -- John. _______________________________________________ Wireshark-dev mailing list Wireshark-dev@xxxxxxxxxxxxx http://www.wireshark.org/mailman/listinfo/wireshark-dev
- Prev by Date: Re: [Wireshark-dev] Teamspeak2 Dissector
- Next by Date: Re: [Wireshark-dev] [PATCH]: enhanced "what's past last mpls label?" heuristic
- Previous by thread: Re: [Wireshark-dev] Teamspeak2 Dissector
- Next by thread: [Wireshark-dev] Build-in
- Index(es):