Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] master b0e6fbf: umts_fp: Replace se_new0
On Wed, Dec 31, 2014 at 1:17 PM, Bill Meier <wmeier@xxxxxxxxxxx> wrote:
> On 12/31/2014 12:52 PM, Bill Meier wrote:
>>
>> On 12/31/2014 11:31 AM, Evan Huus wrote:
>>>
>>> This is an init routine, which can be called when no file is in scope,
>>> so wmem_file_scope() is incorrect (and se_* was also incorrect).
>>>
>>> I'm actually not sure what this routine is doing, since it deals with
>>> conversations but there will never be any conversations e.g. on
>>> startup when the init routine is actually called...
>>>
>>
>> I realized (after I did the commit) that there must be a reason why a
>> just a few ep/se calls remained (all to related to the use of UATs ?).
>> :)
>>
>> This thing looks like it's setting up conversations based upon UAT info
>> prior to dissection (presumably so that any subsequent dissection will
>> be able to take advantage of info stored in the conversation structs).
>>
>> Taking a quick look at things I see that in packet.c the init-routines
>> are called after setting up file scope and initializing the conversation
>> stuff, so it would seem that using file_scope might be OK.
>>
>> se_free_all();
>> wmem_enter_file_scope();
>> ...
>> epan_conversation_init();
>> ...
>> g_slist_foreach(init_routines, &call_init_routine, NULL);
>>
>> Am I missing something ?
>>
>> Are there other cases where the init routines are called out of file
>> scope ?
>>
>> A problem I do see: if the UAT is changed while a file is open, there's
>> no update of the conversation table from the UAT. I don't know if
>> there's something in the UAT info which might change the dissection.
>>
>> Maybe this is why the code checks to see if there'san existing
>> conversation (altho currently there can't be any since the code is only
>> exec'd as an init.
>>
>>
>
> Oops: I see that cleanup_dissection() also calls the init routines (after
> doing conversation_cleanup()), so I guess that trying to add info to the
> conversation structures at this point might not work too well.
>
> I'll have to dig a little deeper to see what really happens now and what
> might be done for all of this.
Ya, the scoping of memory allocated in UAT-related code is a bit of a
mess, because UATs can be edited while no file is open (and thus also
while no packet is being dissected). Ideally any UAT callbacks would
use manual memory (glib directly, or wmem's NULL scope) and then use a
subsequent UAT callback to free it. It's not as simple a conversion as
emem->wmem though, so I've never worked up the willpower to take a
crack at it. I believe (haven't checked) that doing this correctly may
involve adding another callback hook into the UAT code for cleanup
purposes.
