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
