Wireshark-bugs: [Wireshark-bugs] [Bug 2263] new wireshark dissector for SERCOS III (ethertype 0x

Date: Tue, 15 Apr 2008 11:06:05 -0700 (PDT)
http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2263





--- Comment #12 from Jaap Keuter <jaap.keuter@xxxxxxxxx>  2008-04-15 11:05:59 GMT ---
These small issues have been spotted in the patch:
plugins/Makefile.nmake didn't have $(PLUGIN_TARGET) as build target
plugin.rc.in was old, incompatible with 1.0.0
plugins/sercos/Makefile.nmake is outdated
plugins/sercos/Makefile.common: all source files, except packet-sercosiii_1v1.c
must be assigned to the symbol DISSECTOR_SUPPORT_SRC.
All files should have clean $Id$ tokens
Source files must not contain #include "moduleinfo.h"
sprintf() is not safe to use, use g_snprintf() instead

The main problem though is this, in packet-sercosiii.c:

struct siii_mdt_devctrl_info_t*   siii_mdt_devctrl_list = NULL;
struct siii_at_devstat_info_t*    siii_at_devstat_list = NULL;
struct siii_svc_info_t*           siii_mdt_svc_list = NULL;
struct siii_svc_info_t*           siii_at_svc_list = NULL;
struct siii_conn_info_t*          siii_mdt_conn_list = NULL;
struct siii_conn_info_t*          siii_at_conn_list = NULL;
struct siii_phase_info_t*         siii_phase_list = NULL;
struct siii_mdt_svc_idn_t*        siii_mdt_svc_idn_list[MAX_SERCOS_DEVICES] =
{NULL,};

These are used as anchors for lists of device information statically stored in
memory of the dissector. That is a really poor way to store such information. 
The main problem is that this inhibits multithreading and multi capture file
support in Wireshark, a feature we are working towards.
Glib provides you with all kinds of support mechanisms for storing this
information, to efficiently index and search these items. The current methods
(linear search) just don't scale and performance will suffer. 

I'm also unsure what all these structures are used for. It seems overcomplex to
me. My impression is that the use of conversations wasn't known to the original
author.

I'm afraid this mechanism has to change before we can commit this plugin to the
repository.


-- 
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.