Wireshark-bugs: [Wireshark-bugs] [Bug 1897] eDonkey dissector update patch
Date: Fri, 12 Oct 2007 12:56:28 +0000 (GMT)
http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1897





------- Comment #4 from stefano.picerno@xxxxxxxxx  2007-10-12 12:56 GMT -------
(In reply to comment #3)
> I do not know this protocol, but I have some comments to the patch:
> - I think the new value_string's should have a readable string instead of just
> dumping the name of the define.

As you can imagine, the value_string has been automatically generated by a vim
macro applied to the list of defines. 
Basically, instead of 
  KADEMLIA_FINDBUDDY_RES         ,"KADEMLIA_FINDBUDDY_RES"      
you'd like to have
  KADEMLIA_FINDBUDDY_RES         ,"Kademlia Find Buddy Response"      
But is it really better? If it is, I'll do it.

IMHO it's better to display the same keyword you find in the source code, as
source code is the one and only reference for protocol specs.

> - You should use _add_item instead of _add_text for some of your elements (Tag
> Type/Name, and probably more) to make them searchable.

Ok, you're right. 
I began modifying edonkey dissector to debug aMule client.
I was experiencing misterious malfunctioning, so I modified the edonkey
dissector to help me spot communication problems. And it worked! :)
I had no real need to search the tree, I wanted just to decode/inspect packets. 

I'll add some more searchable elements

> - The tag string uses the filter prefix "edonkey" and the tag integer uses the
> filter prefix "kademlia.tag".  I think they should have the same prefix.

This can easily be fixed.
The vanilla edonkey dissector already has this problem for "overnet" packets.
See http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1802 .
I'll fix them both.

> - What about dumping "Name: Value" in the "Tag[1/4]" (and alike) subtree entry?

You mean to obtain something like "Tag[1/4] Name: Value" in the tree? Nice.
However I'd keep all the available infos in sub-elements contained in under
this entry, to let even tag details to be "searchable"

> - How many port numbers do you register in the end of this patch?
> 

Sorry, I forgot at the end of the patch a set of debugging/testing code.
I'll remove the extra port number registrations.

BTW, Is there a way to configure permanently (ie: in some cfg file) to decode
packet on a certain port with a specific protocol? (ie: make persistent the
effect of DecodeAs)


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