Wireshark-bugs: [Wireshark-bugs] [Bug 2798] New Dissector: IEEE C37.118 Synchrophasor Protocol
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2798
--- Comment #3 from Jens Steinhauser <jens.steinhauser@xxxxxxxxxx> 2008-08-12 06:44:35 PDT ---
(In reply to comment #2)
Thanks for commenting.
> - Having empty filter fields makes Chris very unhappy. Please fill them out
fixed. I removed unneeded (reserved) fields and filled out the other ones.
> - No need to use (current_udp_port != -1), you have 'initialized' for that.
fixed.
> - If have have to format time to UTC, then mark it as such. Add 'UTC' to it.
the formatted string is added with 'proto_tree_add_string(, hf_soc, ...)', and
'hf_soc's name is "SOC time stamp (UTC)". This way it also shows up in the
filter expression dialog.
> - g_assert has no place in a dissector, use DISSECTOR_ASSERT macro instead.
the 'offset' value is counted up from 0 to 14, always, no ifs, no loops. But
OK, I removed it.
> - /* FNOM and CFGCNT */ can easily be converted in proto_tree_add_item() calls.
fixed
> - The inline keyword does not fly with regular C compilers.
fixed, good compiler should inline anyway.
> 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.
I developed the dissector as a plugin for the obvious reasons, but I have no
doubts against converting it to a regular dissector.
> 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
the 'get_pdu_length()' callback doesn't need all parameters, and that causes an
error. Does WS have macros to add something like gccs '__attribute__((unused))'
for various compilers? Omitting the parameter name is only valid in c++, IIRC.
> We'll have to preserve the patch of libwireshark.def since this seems required.
another reason for a regular dissector, no need to modify the def file then.
Jens
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
