Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 5990] SERCOS III built-in dissector (from plugin)

[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5990

Roland Knall <rknall@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rknall@xxxxxxxxx

--- Comment #2 from Roland Knall <rknall@xxxxxxxxx> 2011-06-04 08:06:32 PDT ---
(In reply to comment #1)
> Created an attachment (id=6450)
 --> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=6450) [details]
> SERCOS III dissector source in epan/dissector directory
> 
> This source shouldn't need to be added as a patch, it should just be moved from
> the plugins/sercosiii directory to the epan/dissectors directory.

I do have some suggestions though. 

1. Why still keep packet-sercosiii.h? The only definitions in there, which
should be globally available throughout the dissector code are:

#define MAX_SERCOS_DEVICES (512)

#define COMMUNICATION_PHASE_0 (0x0)
#define COMMUNICATION_PHASE_1 (0x1)
#define COMMUNICATION_PHASE_2 (0x2)
#define COMMUNICATION_PHASE_3 (0x3)
#define COMMUNICATION_PHASE_4 (0x4)

2. In total the whole dissector code has 1973 lines. There are quite a few
other dissectors with more lines of codes.

I would suggest, combining the files into one, and adding the dissector as a
whole. Right now, there are 1315 files in the dissectors sub directory.
Unnecessarily adding 9 more, where 1 should suffice, seems rather pointless.

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