Hi,
Comments:
1,
You should not use that DIS_BitMask structure.
You should instead use
explicit true_false_string and FT_BOOLEAN if it is a boolean or
value_string if it is not a boolean.
This makes the filter gui much nicer.
See packet-ip.c for examples on how to do this.
2,
All these DIS_ParserNodes should also have their own
value_string associated with it and the corresponding hf_field should
have a VALS(that_value_string) associated with it.
See multiple examples on how to use VALS and value_strings in the code.
See also examples for how to use val_to_str() to retreive the string
from a value_string if you want to put it in the info column or
something.
Again, having them as proper value_string's will make the filter giu
much nicer.
3, dont use a pointer to offset.
All other dissectors use offset explicitely.
Helper functions in dissector take offset as a parameter and return
the new value for offset.
offset = call_some_helper( ... , offset, ...);
Change it to be consistent with the rest of ethereal so that it will
be easier to maintain.
4, you use too many proto_tree_add_text() change all these into
proto_tree_add_item() and define proper hf fields for them so that
filtering will work.
this will make the dissectyor much better.
You wont need to do silly things such as parseField_Int either. these
will go away when using proto_tree_add_item properly.
5,
change all the places where you do
type* variable;
into
type *variable;
to be more consistent with ethereal coding style.
if you can do these changes which are likely to be very intrusive on
the patch it will be much easier to review it.
On Tue, 05 Apr 2005 11:15:32 -0400, "Ouellette, Jeremy J
(N-Scientific Research Corp)" <jeremy.j.ouellette@xxxxxxxx> wrote:
> I've attached an Ethereal submission to dissect Distributed Interactive
> Simulation (DIS) packets, as defined in IEEE-1278. I've done my best to
> follow the Ethereal coding guidelines as I understand them. The patch
> file was generated against revision 14018 in the svn repository (the
> latest as of a few minutes ago). The packet-dis.c header text contains
> a few comments on what's currently included and some ideas for
> enhancement / next steps.
> Also included in the tarball is a sample capture. Note that the UDP
> port for the DIS packets in this capture is 3001 instead of the default
> 3000, so the correct port must be specified in the DIS preferences to
> properly view this capture.
>
> Jeremy Ouellette
> Scientific Research Corporation
>
> <<DIS_dissector.tgz>>
>
>
