Wireshark · Ethereal-dev: Re: [Ethereal-dev] About addition new module for ethereal.

[ronnie sahlberg <ronniesahlberg@xxxxxxxxx>]

Hi   after a very quick review  there are some things i think you
should address:


1,
You have to end all value_strings with a terminating
  {0, NULL }
entry or things like val_to_str() and the dissection of the hf field
containing VALS(<value_string>) may reference values outside of the
array.


2,
In dissect_kink()
+  /* It shows kink type by the type value. */
+  if(check_col(pinfo->cinfo, COL_INFO)){
+    switch(type){
+    case KINK_TYPE_RESERVED:
+      col_add_fstr(pinfo->cinfo, COL_INFO, "%s", "RESERVED");
+      break; 
You should not use a switch here to explicitely print each one of
them,   use a single col_add_fstr()
and val_to_str(kink_type_vals, ...)
There are many examples on val_to_str() in the source.


3,
in dissect_kink()
+  /* Make the kink tree */
+  if(tree){
This if statement should only encapsulate  only
creating the item and the kink_tree   not the actual dissection of the fields
or else it will not be possible to do filtering properly.
I.e.
/* Make the kink tree */
if(tree){
   ti = proto_tree_add_item(tree, proto_kink, tvb, offset, -1, FALSE);
   kink_tree = proto_item_add_subtree(ti, ett_kink);
}
Dont forget to initialize it and kink_tree to NULL where these
variables are declared.


4,
down in print_next_payload()
+print_next_payload(tvbuff_t *tvb, int offset, guint8 next_payload,
proto_tree *tree){
+  switch(next_payload){
+  case KINK_DONE:
Here as well,   declare a proiper value_string and use val_to_str() to
find the correct string to print.


5,
dissect_kink()  should be a new style dissector instead
i.e.  it should be static int  and not static void.
Then it should at the beginning (before any trees or items are
dissected or put into the tree)
check the header that it looks like a valid kink message and return 0   if not.
This will allow ethereal to better distinguish between what is kink
and what is not  especially if the packets are going between two
well-known ports.
changing to a new-style dissector will also require you to change the
proto_reg_handoff_xxx slightly.
(new style dissectors use new_create_dissector_handle() instead )



best regards
  ronnie sahlberg


On Fri, 24 Dec 2004 18:53:47 +0900, T.Nakashima@xxxxxxxxxxxxxxx
<T.Nakashima@xxxxxxxxxxxxxxx> wrote:
> Dear all
> 
> I'm writing to adding new module. I developed new module which can
> analize
> KINK(Kerberized Internet Negotiation of Key).I referred to
> "draft-ietf-kink-kink-jp
> -04.txt, v 1.14 2003/02/10" and implemented it. Would you add this
> module to
> ethereal? It is attached by this mail.
> 
> sincerely.
> ----------
> Yokogawa Electric Corporation
> Name: Takeshi Nakashima
> URL: http://www.yokogawa.co.jp
> 
> 
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
> 
> 
> 
>