[ronnie sahlberg <ronniesahlberg@xxxxxxxxx>]

Kyle,

I have attached a new version of the dissector that uses heuristics to
determine if it is CIGI or not.
No need for specifying a range of ports.

Can you have a look at maybe sharpening the heuristics if possible and comment?



On 12/2/05, Harms, Kyle J <Kyle.J.Harms@xxxxxxxxxx> wrote:
> I have a few questions before I look at this some more.
>
> 1,
> The reason I did Booleans this way is because if I use the FT_BOOLEAN it
> only shows '. = IG Mode...' in the window as apposed to '.... ...1 = IG
> Mode...' which is very useful in debugging CIGI.  Is there a way to use
> FT_BOOLEAN and still see which bits it is actually using i.e. '....
> ...1' ?
>
> 2,
> README.developer does not mention anything about a new style dissector.
> Where might I look at an example of such a dissector?  I might suggest
> that the README.developer stub be updated to be a new style dissector.
>
> 3,
> I don't think there is really a clean way to do a 'range' check in a
> case/switch statement, other than default,  but I need that for else.
>
>         } else if ( packet_id >= CIGI2_PACKET_ID_USER_DEFINABLE_MIN &&
> packet_id <= CIGI2_PACKET_ID_USER_DEFINABLE_MAX ) {
>             hf_cigi2_packet = hf_cigi2_user_definable;
>             packet_length = packet_size;
>         } else {
>             hf_cigi2_packet = hf_cigi_unknown;
>             packet_length = packet_size;
>         }
>
> Is there such a way to make this clean as apposed to
> case 236:
> case 237:
> case 238:
> case 239:
> ...
> case 255:
>
> ?
>
> I will try and address these issues as soon as I can.
>
> Kyle
>
> -----Original Message-----
> From: ronnie sahlberg [mailto:ronniesahlberg@xxxxxxxxx]
> Sent: Friday, December 02, 2005 3:41 PM
> To: Ethereal development
> Subject: [Ethereal-dev] Re: New dissector for CIGI
>
> Hi,
>
>
> 1,
> In some places you specify booleans in a bit suboptimal way:
> Please see :
> the definition of cigi_valid_vals[]   and
> hf_cigi3_ig_control_timestamp_valid.
>
> These ones should instead be using a true_false_string   and the
> hf_field should be    TFS(&cigi_valid_tfs), and be of type FT_BOOLEAN.
>
>
> 2,
> we have many many dissetors now and we get more and more "collissions"
> when ehtereal takes the wrong dissector.
>
> Can you change dissect_cigi()   to being a new-style dissector (i.e.
> returning int) and using new_create_dissector_handle() to register the
> handle.
> The new signature for dissecvt_cigi() would then be static int.
>
> Then dissect_cigi would return 0 if it was not a cigi packet (and
> letting ethereal try something else) or x   for x number of bytes
> eaten by the dissector.
>
> in the beginning of dissect_cigi()  before you start manipulating
> col_info/col_protocol and before starting dissection  you should add
> some heuristics to verify (as far as this is possible) that this does
> indeed look like a cigi packet.  and return 0 if not.
>
>
> 3,
> you use very long chains of if()  {} else if {} when demultiplexing
> things like packet_id and calling the subdissectors.
> change this to a switch/case
>
>
>
> apart from that the dissector looks good
> can you please address the 3 issues above?
>
>
>
>
>
> On 12/2/05, Harms, Kyle J <Kyle.J.Harms@xxxxxxxxxx> wrote:
> > Hi,
> >
> > This patch is for a CIGI dissector (complete versions 2 and 3).  It
> has
> > been [fuzz] tested on GNU/Linux using the Ethereal 0.10.13 codebase.
> > However, the patch here is against the svn repository.
> >
> > More information about CIGI can be found at
> http://cigi.sourceforge.net/
> >
> > Kyle Harms
> >
> >
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>

Attachment: packet-cigi.c.gz
Description: GNU Zip compressed data