Hi,
A fix is already in, see
http://anonsvn.wireshark.org/viewvc/index.py/trunk/epan/conversation.c?r1=24661&r2=25158
Thanx,
Jaap
Jens Steinhauser wrote:
> The attached patch fixes conversation_lookup_hashtable().
>
> Thanks,
> Jens
>
> On Wed, 2008-04-23 at 22:17 +0200, Jaap Keuter wrote:
>> Hi,
>>
>> Yes, I think you're on the right track here.
>>
>> Concerning the conversation search, I think you've a point. When searching
>> for
>> a conversation along the time axis, you shouldn't get the a conversation
>> before the first one is established.
>>
>> I'm not aware if many dissectors use conversations that way and this is a
>> corner case. That may be why it wasn't spotted before.
>>
>> A simple fix for your code is to check the returned conversation frame
>> number
>> against the current frames' number and discard it when it's older. Of course
>> that should be done by the search routine, for which a change will be
>> committed later.
>>
>> Thanx,
>> Jaap
>>
>> Jens Steinhauser wrote:
>>> 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
>>>>>
_______________________________________________
Wireshark-dev mailing list
[email protected]
http://www.wireshark.org/mailman/listinfo/wireshark-dev
