Wireshark · Wireshark-dev: Re: [Wireshark-dev] Another one with dissector questions

[Jens Steinhauser <jens.steinhauser@xxxxxxxxxx>]

Ok, I changed my dissector to use a conversation. The dissector creates
a new conversation for every configuration frame it finds and uses
conversation_add_proto_data() to save the information that is needed to
dissect the data frames. When it dissects a data frame, it uses
find_conversation() and conversation_get_data() to get the information
from the config frame. Is that the proper way?

But strange stuff happens when I have data frames before the first
configuration frame:

README.developer says (line 2741): "The conversation returned is where
                          (frame_num >= conversation->setup_frame
                        && frame_num < conversation->next->setup_frame)"
and (line 2723): "If no conversation is found, the routine will return a
NULL value."

It is my understanding that find_conversation() should return NULL for
the data frames before the configuration frames.

find_conversation() calls conversation_lookup_hashtable() to search for
the conversation (conversation.c, line 632):

match = g_hash_table_lookup(hashtable, &key);
  
if (match) {
   for (conversation = match->next; conversation; conversation =
conversation->next) {
      if ((conversation->setup_frame <= frame_num)
       && (conversation->setup_frame > match->setup_frame))
         match = conversation;
   }
}
 
return match;

The code doesn't work when frame_num is smaller than setup_frame of all
conversation in the linked list. The "for (...; conversation; ...)"
doesn't execute the body of the loop because conversation is null and
returns a conversation with setup_frame bigger than frame_num passed to
find_conversation(). I think that's a bug.

Thanks,
Jens


On Tue, 2008-04-22 at 15:58 +0200, Jaap Keuter wrote:
> Hi,
> 
> 1) You better use a conversation. Read README.developer on what
> conversations are and how they are designed for specifically this purpose.
> Using globals is never a good way to do this, especially when we intend to
> multithread/work with multiple files.
> 
> 2) ett's are datastructures for subtrees. They hold the expanded/collapsed
> state for instance. You can reuse them, but then all these subtrees will
> have the same expanded/collapsed state.
> 
> 3) Remove all display filters and *disable packet coloring* (which is also
> display filter based) to get tree==NULL.
> 
> Thanx,
> Jaap
> 
> > Hi,
> >
> > I'm working on a dissector for about a month and some questions came up
> > when doing more than just basic dissection of the packets:
> >
> > 1) The protocol consists mainly of two different types of packets:
> > configuration frames and data frames. Configuration frames are send at
> > the beginning of a transmission or when the configuration of the sending
> > device changes and describe the content and format of data frames.
> > Without the knowledge out of the configuration frames, the data frames
> > can't be dissected.
> >   So I created a struct, called config_frame to save the information of
> > the configuration frames and put these config_frames into a global
> > GArray. This global array is (re)initialized in my dissectors init
> > function and pinfo->fd->num (also saved in config_frame) is used to
> > ensure that every configuration frame just adds one config_frame struct
> > to the array. When the dissector comes to a data frame, he searches
> > through that global array for a matching config_frame and uses it to
> > dissect the data frame. Is that the right way to do that?
> >
> > 2) What are the ett_... values for? Section 1.6.2 of README.developer
> > explains how to initialize and use them, but I didn't get what they are
> > good for. Is it save to pass the same ett_ variable multiple times? (The
> > data in the data frames is nested and I do that, didn't have a problem
> > so far)
> >
> > 3) To understand the sequence in that the "proto_register_...", the
> > "proto_reg_handoff_...", the init and the dissect functions are called,
> > I put the following at the beginning of the dissect function:
> >
> >    printf("dissect(): %s\n", (!tree ? "*tree null" : "*tree valid"));
> >
> >    and always get "*tree valid". Do i need a special configure switch to
> > enable the "Operational dissection" (enable something like a release
> > build)?
> >
> > Thanks,
> > Jens
> >
> >
> > *****************************************************************
> > Jens Steinhauser (Mr.), Software Dude
> > Tel.: +43 5523 507 422, Fax.: +43 5523 507 999
> > http://www.omicron.at/  jens.steinhauser AT omicron.at
> > *****************************************************************
> > OMICRON electronics GmbH, Oberes Ried 1
> > A-6833 Klaus / Austria
> > Company Registration No. FN 34227i, Commercial Court of Feldkirch
> > *****************************************************************
> >
> > _______________________________________________
> > Wireshark-dev mailing list
> > Wireshark-dev@xxxxxxxxxxxxx
> > http://www.wireshark.org/mailman/listinfo/wireshark-dev
> >
> 
> 
> _______________________________________________
> Wireshark-dev mailing list
> Wireshark-dev@xxxxxxxxxxxxx
> http://www.wireshark.org/mailman/listinfo/wireshark-dev
-- 
*****************************************************************
Jens Steinhauser (Mr.), Software Dude
Tel.: +43 5523 507 422, Fax.: +43 5523 507 999
http://www.omicron.at/  jens.steinhauser AT omicron.at
*****************************************************************
OMICRON electronics GmbH, Oberes Ried 1
A-6833 Klaus / Austria
Company Registration No. FN 34227i, Commercial Court of Feldkirch
*****************************************************************