Wireshark-bugs: [Wireshark-bugs] [Bug 2798] New Dissector: IEEE C37.118 Synchrophasor Protocol
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2798
--- Comment #2 from Jaap Keuter <jaap.keuter@xxxxxxxxx> 2008-08-12 04:32:19 PDT ---
First of all I must say: It's a very well prepared patch.
A quick look generated some remarks, though:
- Having empty filter fields makes Chris very unhappy. Please fill them out
with
reasonable values.-
- No need to use (current_udp_port != -1), you have 'initialized' for that.
- If have have to format time to UTC, then mark it as such. Add 'UTC' to it.
- g_assert has no place in a dissector, use DISSECTOR_ASSERT macro instead.
- /* FNOM and CFGCNT */ can easily be converted in proto_tree_add_item() calls.
- The inline keyword does not fly with regular C compilers.
For the rest I'm tempted to recommend it to be collapsed into a regular
dissector. This might also takes care of the math lib dependency, which I
believe exists, and am not sure how that work out on all platforms.
Furthermore it should be possible to make it compile cleanly. Could you
elaborate on the comment
# this plugin won't compile w/o warnings because
# of unused parameters in the callbacks
We'll have to preserve the patch of libwireshark.def since this seems required.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
