Wireshark-dev: Re: [Wireshark-dev] Kismet protocol dissector
From: "ronnie sahlberg" <ronniesahlberg@xxxxxxxxx>
Date: Thu, 13 Jul 2006 08:49:48 +0000
Checked in.

Nice work!


Oh,   on the wiki, i forgot to specify this before, sorry for that
but we usually put all example capture on
wiki.wireshark.org/SampleCaptures  and then link to these captures
from the protocol page instead of putting the actual capture on the
protocol page.
This pust all captures in a singler place for those that want to
quickly scan what captures are available and to play with.
my mistake.

can you move them to /SampleCaptures and link there from the protocol page ?



You put the braces for compound statement differently from the rest of
wireshark coding style
which (most of the time/often) is
  if (expression)  {

this will make it harder for us to maintain your dissector.

at your convenience   could you look at changing the brace placement?




I also belive that the port was "assigned" (randomly picked) by the
Kismet's author :-)

Ports only become WellKnown if it is IANA that assigns them.
So it shouldnt be referred to as WellKnown in the case of kismet,
using "default port" is ok though.




On 7/13/06, Krzysztof Burghardt <krzysztof@xxxxxxxxxxxx> wrote:
> However,   port 2501 is registered for the rtsclient protocol and unless
> kismet is the same as rtsclient
> it would be incorrect to refer to this as  a well-known port for kismet.
> further down on the page the port is referred to as the default port?

So this is probably not a registered port. I think it was assigned by
Kismet's author.

> if it is a default port  you should add the port as a preference setting
> which defaults to 2501  but can be changed by the user.

Yes, users can change port. I made it a preference.

> Kismet is an ASCII based protocol?

Yes, it is.

>  If so you may check that the first 8 bytes of the tvb (if there are 8
> bytes or more) are actual ascii values  >32 <128  ?

Done.

> You have a lot of
> offset += next_token - line; linelen -= next_token - line; line =
next_token;
> can you break these up to one assignment/statement per row  and add a
blank

All done. Patch attached.
Regards,
--
Krzysztof Burghardt <krzysztof@xxxxxxxxxxxx>
http://www.burghardt.pl/