Wireshark-bugs: [Wireshark-bugs] [Bug 3467] Memcache Textual Protocol dissector patch
Date: Mon, 18 May 2009 13:50:25 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3467


Stig Bjørlykke <stig@xxxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |stig@xxxxxxxxxxxxx




--- Comment #4 from Stig Bjørlykke <stig@xxxxxxxxxxxxx>  2009-05-18 13:50:19 PDT ---
First review:

In frame 6 and 15 in your example we get two tree items with "Memcache
Protocol", the latest with only \r\n, which I think is wrong because we only
have one command.  Are you able to fix this?

In frame 25, 31 and 32 it seems like some continuation packages, but the does
not show up correct.  Is this due to the dissector or the sent packages?

The binary dissector is using expert info's to provide info when something is
wrong.  Maybe you are able to add this for the text dissector?

I also find some "goto" statements which could easily be omitted by rewriting
the code.  Personally I dont like goto because they obfusticate the code flow.

It's difficult to read a patch with a lot of whitespace changes.  I have
rewritten some of the code in the first patch (coding style is like clothing
style, but I prefer having the same style in one file), so you can have a look
at the latest patch.


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