Wireshark-bugs: [Wireshark-bugs] [Bug 3519] Enhancement of the SERCOS III dissector plug-in
Date: Tue, 15 Sep 2009 13:02:07 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3519





--- Comment #7 from Bill Meier <[email protected]>  2009-09-15 13:02:06 PDT ---
OK: I've tried the patch in my environment. (It took a little manual effort
since several of the source files have changed since the diff was generated).

It compiles (on my Windows environment) and seems to work.

I've not looked at the code in detail.

One minor question: Looking at the capture provided: All the 'phase change'
packets other than those with 'COMMUNICATION_SWITCH_3' as the 'phase'
show "CP is Unknown". Is this correct ?

I'm not familiar with the sercosiii protocol so I can't otherwise
particularly comment as to the details of the protocol dissection.

-----------

The above being said I note that siii_sub_devices is a statically
allocated array which is appears to be about 1.8 Meg in size.

It seems to me that this is too large of a static memory allocation to be 
present all the time and whether or not there are actually any frames in a
capture for this dissector.

I think a some kind of deferred dynamic memory allocation for this
(and related ?) arrays might be better;
 However I'm not sure as to the details as to how best to do this.

I would expect that one of the other developers can can comment on this and
maybe suggest a good approach.


I do note what seems to be a related comment re static allocation by 
Jaap Keuter when the original sercosiii dissector was committed.

See https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2263#c12


-----------
>From packet-sercosiii.h (condensed)

typedef struct _siii_sub_device_t {
  guint16 mdt_c_dev_offset; /* ...
  guint16 at_s_dev_offset; /* ...
  guint16 mdt_svc_offset; /*  ...
  guint16 at_svc_offset; /* ...
  siii_connection_t siii_subdev_cons[MAX_CON_PER_SUBDEVICE]; /* ...
} siii_sub_device_t;


>From packet-sercosiii_1v1_svc.c (condensed)

siii_sub_device_t siii_sub_devices[MAX_SERCOS_DEVICES+1];

...

static guint16 siii_last_idn[MAX_SERCOS_DEVICES]; /* The transmitted ...
static guint8 siii_last_index[MAX_SERCOS_DEVICES]; /* The transmitted  ... 
static guint8 siii_last_element[MAX_SERCOS_DEVICES]; /* The transmitted ...
...


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