Wireshark-dev: Re: [Wireshark-dev] Kismet protocol dissector
From: "ronnie sahlberg" <[email protected]>
Date: Wed, 12 Jul 2006 11:15:14 +0000
Hi,

Some comments

1, please create a WIKI page on wiki.wireshark.org for this protocol.
Fill it out as much as possible to make it as nice and informative as
possible for causal non-kismet expert users

2, upload the example traces to that wiki page.

3, you use "hidden" fields such as the hf_kismet_[request|response]
Hidden fields are in general a bad idea since it prevents the user
from knowing about them.
Please redo these as "generated" fields instead   so they are visible
in the trace and the users will find out about them and can use them.
Please grep for PROTO_ITEM_SET_GENERATED for examples on how to
display "generated" fields as "generated".
Note: it often makes sense to try to put all "generated" fields at the
top of the expansion where they reside.

4, you usae tvb_get_string   whis requires the data to ge g_free()d
which you do!   but it would be better to switch to something like
tvb_get_ephemeral_string() that does not require an explicit free
and thus would not leak memory if an exception is caused between teh
tvb_get_string() andf the g_free() occurs.


5,   is it possible to change the dissector to be a new style
dissector   returning gboolean instead of an oldstyle returning void?
please see mailinglist archive for reasons why new-style is better
than oldstyle.



please address these issues and resubmit the patch to the mailinglist
for rereview.


best regards
ronnie s




On 7/12/06, Krzysztof Burghardt <[email protected]> wrote:
Hello,

I made Kismet protocol dissector for Ethereal for Client/Server protocol.
Kismet Drone/Server protocol still needs separate dissector, because it
uses different protocol.

Patch for current SVN revision (e.g. 18189) attached.

More information, including protocol traces can be found here:
http://www.burghardt.pl/wiki/software/kismet_protocol_dissector_for_ethereal

Regards,
--
Krzysztof Burghardt <[email protected]>
http://www.burghardt.pl/