Re: [Wireshark-dev] [Wireshark-commits] master b0e6fbf: umts_fp: Replace se_new0(...) by wmem_new0(wmem_file_scope(), ...)
On Wed, Dec 31, 2014 at 1:17 PM, Bill Meier 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. ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master b0e6fbf: umts_fp: Replace se_new0(...) by wmem_new0(wmem_file_scope(), ...)
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. Bill ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master b0e6fbf: umts_fp: Replace se_new0(...) by wmem_new0(wmem_file_scope(), ...)
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. Bill ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] master b0e6fbf: umts_fp: Replace se_new0(...) by wmem_new0(wmem_file_scope(), ...)
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... On Wed, Dec 31, 2014 at 11:11 AM, Wireshark code review wrote: > URL: > https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=b0e6fbf2d4ac4c5819d09b2a5072e74436b4f1c8 > Submitter: Bill Meier (wme...@newsguy.com) > Changed: branch: master > Repository: wireshark > > Commits: > > b0e6fbf by Bill Meier (wme...@newsguy.com): > > umts_fp: Replace se_new0(...) by wmem_new0(wmem_file_scope(), ...) > > Change-Id: I9d40ffd199147fb8b975c493253d5cf796be5983 > Reviewed-on: https://code.wireshark.org/review/6179 > Reviewed-by: Bill Meier > > > Actions performed: > > from f7b6dcc Lua: allow a Dissector object to be passed in for > register_heuristic > adds b0e6fbf umts_fp: Replace se_new0(...) by > wmem_new0(wmem_file_scope(), ...) > > > Summary of changes: > epan/dissectors/packet-umts_fp.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > ___ > Sent via:Wireshark-commits mailing list > Archives:http://www.wireshark.org/lists/wireshark-commits > Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits > > mailto:wireshark-commits-requ...@wireshark.org?subject=unsubscribe ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
